-
Notifications
You must be signed in to change notification settings - Fork 38
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 an iris-esmf-regrid based regridding scheme #2457
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2457 +/- ##
==========================================
+ Coverage 94.70% 94.72% +0.02%
==========================================
Files 248 249 +1
Lines 14002 14071 +69
==========================================
+ Hits 13260 13329 +69
Misses 742 742 ☔ View full report in Codecov by Sentry. |
55d40ce
to
642e0a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good already Bouwe, it's great to see our custom ESMF code being replaced. Some additional comments:
- In the PR description, please replace "unstructured" with "irregular" (you are replacing the schemes for irregular)
- Would it make sense to deprecate our own
ESMFPy...
schemes? I don't think they are really useful anymore given that we have a much better solution for this now. - Would it maybe even make sense and go one step further by also replacing the schemes for regular regridding? This would make our code even easier. I think
esmf-regrid
also supports that kind of regridding with the same schemes. In principle, it would even work for unstructured grids, but in this case the data would need to be UGRID-compliant, and I am not sure we can enforce that (yet).
Thanks for reviewing @schlunma!
I mixed those two up, it should be better now.
Yes, but maybe with the next release of iris-esmf-regrid where the
I like this idea, but there is an issue with ESMF that prevents parallelisation using Dask (see #2316). I have reported it to the ESMF developers and they said they saw opportunities to solve it, but I'm not sure when this will be solved. I would be keen to improve our parallisation scheme for those cases where ESMF is not needed and then it is relevant to only use it when is really needed. |
Thanks for addressing my comments @bouweandela! Could you please add some documentation about the new schemes for meshes? It would also be great to use a common naming convention for this, e.g., at the moment, the documentation only talks about UGRID instead of iris meshes (in the ICON doc). Maybe you could move that part to regridding docs and only reference that in the ICON-specific section? |
…s-esmf-regrid-scheme
…s-esmf-regrid-scheme
Done in 9aaf1b2 |
@schlunma I've improved the documentation and testing and also added support for lazy nearest neighbour regridding now because this is at least partly supported with the new iris-esmf-regrid v0.11. Could you do another review, please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! I just tested this extensively with all different grid types, and everything works as excepted 🚀
Just have a couple of minor comments on the doc. Cheers!
Co-authored-by: Manuel Schlund <[email protected]>
Co-authored-by: Manuel Schlund <[email protected]>
Thanks for reviewing @schlunma. I think I have addressed all comments now, could you have another look please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks @bouweandela! This will facilitate regridding enormously 🚀
Description
Add an iris-esmf-regrid-based regridding scheme and use it as the default scheme for irregular grids and meshes
Example usage (as generic scheme) in preprocessor in recipe:
Example plots for regridding 2D variable
tos
to a 1x1 degree grid are available here.Example plots for regridding 3D variable
thetao
to a 1x1 degree grid and subsequently extracting the 500m depth level are available here.Closes #2405.
Links to documentation:
esmvalcore.preprocessor.regrid_schemes.IrisESMFRegrid
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.
To help with the number pull requests: