-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add recipe for sea ice area and extents in southern polar region #3607
base: main
Are you sure you want to change the base?
Conversation
@rbeucher see draft pr for sea ice recipe |
this recipe could possibly be moved to examples ? |
@headmetal @rhaegar325 can you also have a look at this when you get a chance? |
@flicj191 Could you please have a look at the checklist and make sure you've addressed all the items as best as you can? Our convention is that the author of the pull request fills in the checklist before asking for a review. If anything is unclear don't hesitate to ask. |
Codacy issues were too many local variables and a docstring that matches PEP257 from what I can see. Are these acceptable? @ESMValGroup/esmvaltool-developmentteam |
Yes, a few issues should be fine. |
esmvaltool/diag_scripts/seaice_area_extents/seaice_mapextents.py
Outdated
Show resolved
Hide resolved
esmvaltool/diag_scripts/seaice_area_extents/seaice_mapextents.py
Outdated
Show resolved
Hide resolved
esmvaltool/diag_scripts/seaice_area_extents/seaice_mapextents.py
Outdated
Show resolved
Hide resolved
esmvaltool/diag_scripts/seaice_area_extents/seaicearea_trends.py
Outdated
Show resolved
Hide resolved
esmvaltool/diag_scripts/seaice_area_extents/seaicearea_trends.py
Outdated
Show resolved
Hide resolved
esmvaltool/diag_scripts/seaice_area_extents/seaice_mapextents.py
Outdated
Show resolved
Hide resolved
esmvaltool/diag_scripts/seaice_area_extents/seaice_mapextents.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Anton Steketee <[email protected]>
Co-authored-by: Anton Steketee <[email protected]>
Co-authored-by: Anton Steketee <[email protected]>
The choice for xarray here seems mostly a personal preference and that is absolutely fine! However, re-implementing existing preprocessor functions in the diagnostic is something that should preferably be avoided. |
esmvaltool/diag_scripts/seaice_area_extents/seaice_mapextents.py
Outdated
Show resolved
Hide resolved
In general, I tend to agree with this, but this may lead to a number of issues:
That's why I asked Felicity, if possible, to stick with the oldschool Iris way 🍺 |
I agree with @valeriupredoi and @bouweandela that the preprocessors should handle the heavy lifting. Since they're based on IRIS, we're somewhat limited at the moment. Supporting XArray in the future would be great, though I understand it's a complex task. |
@rbeucher Please open a GitHub issue if you encounter such limitations. We are applying for funding for future work at the moment, so if we are aware of shortcomings in the preprocessor, we can plan to solve these issues in future projects. |
Sure, I will. We haven't worked on this recently. Hopefully soon. |
Hi @flicj191 if you are still looking for a science reviewer I could ask one of the sea-ice team at the Met Office. I would think that @anton-seaice's review would suffice for science review since he knows what the outputs should look like. But let me know if you think it really needs another science reviewer. |
Thanks @alistairsellar ! I will do a bit more work on this to test some regridders then will get @anton-seaice to have a look at the outputs so sounds good for that to be the science review. Cheers |
Sounds good @flicj191 ! |
Hi @bouweandela @valeriupredoi I have restructured the recipe to use the preprocessors ready for re-review Checking some different regridding preprocessors, This is a plot from previous diagnostic script
then with this preprocessor: map_extents: &map
climate_statistics:
operator: mean
period: monthly
extract_region:
start_longitude: 0
end_longitude: 360
start_latitude: -90
end_latitude: -50
regrid:
target_grid: 0.5x0.5
scheme: linear map_esmpy:
<<: *map
regrid:
target_grid: 0.5x0.5
scheme:
reference: esmf_regrid.schemes:ESMFBilinear
map_target:
<<: *map
regrid:
target_grid: NSIDC-G02202-sh
scheme: linear map_esmpy_target:
<<: *map
regrid:
target_grid: NSIDC-G02202-sh
scheme:
reference: esmf_regrid.schemes:ESMFBilinear
mdtol: 0.7
|
I expect bilinear interpretation is a good choice. The monthly averages of sea ice concentration should be fairly 'smooth' in space, so biliear is appropriate. |
Description
New recipe for sea ice area trends and extents around Antarctica. Using code from @anton-seaice for a COSIMA (Consortium for Ocean-Sea Ice Modelling in Australia) 'cookbook' example.
Before you get started
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.
Any changed dependencies have been added or removed correctlyNew or updated recipe/diagnostic
To help with the number of pull requests: