Generic regridding preprocessor

[zklaus]

Description

This adds the long-promised iris-esmf-regrid based regridding to the regrid preprocessor.
It does so in a generic way, meaning the preprocessor is not actually adapted to the iris-esmf-regrid package, but rather gets the ability to load any kind of Iris regridding scheme by specifying the corresponding factory, also allowing for the passing of arbitrary arguments.

I'd like to solicit some early feedback!

Do you think this is feasible, i.e. using a generic loading mechanism?

If you want to try it out, don't forget to create a new environment that contains the iris-esmf-regrid conda package as mentioned in the environment.yml in this PR (of course you can also just install it in your environment).
You can use the corresponding branch iris-esmf-regrid and the recipe_ocean_multimap.yml in it to try things out.

Documentation:

• Preprocessor docs: https://esmvaltool--1448.org.readthedocs.build/projects/ESMValCore/en/1448/recipe/preprocessor.html#regridding-interpolation-extrapolation-schemes
• API docs: https://esmvaltool--1448.org.readthedocs.build/projects/ESMValCore/en/1448/api/esmvalcore.preprocessor.html#esmvalcore.preprocessor.regrid

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.

• 🧪 The new functionality is relevant and scientifically sound
• 🛠 This pull request has a descriptive title and labels
• 🛠 Code is written according to the code quality guidelines
• 🧪 and 🛠 Documentation is available
• 🛠 Unit tests have been added
• 🛠 Changes are backward compatible
• 🛠 Any changed dependencies have been added or removed correctly
• 🛠 The list of authors is up to date
• 🛠 All checks below this pull request were successful

---

To help with the number pull requests:

• 🙏 We kindly ask you to review two other open pull requests in this repository

[codecov]

Codecov Report

Merging #1448 (3469753) into main (a02732d) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 3469753 differs from pull request most recent head 17428ab. Consider uploading reports for the commit 17428ab to get more accurate results

@@            Coverage Diff             @@
##             main    #1448      +/-   ##
==========================================
+ Coverage   90.75%   90.76%   +0.01%     
==========================================
  Files         197      197              
  Lines       10552    10568      +16     
==========================================
+ Hits         9576     9592      +16     
  Misses        976      976              

Impacted Files	Coverage Δ
esmvalcore/preprocessor/_regrid.py	97.09% <100.00%> (+0.15%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a02732d...17428ab. Read the comment docs.

[schlunma]

Nice! I really like the idea of this generic loading mechanism! By doing this we offer the user a large amount of flexibility in terms of module used, scheme used, keywords used, etc.

I will give it a try tomorrow 👍

[schlunma]

I just successfully tested this PR with a couple if iris schemes (e.g. iris.analysis.Linear) 🎉 It works really well and just like expected.

Three small comments:

• If an invalid scheme is provided, the error message can be really cryptic. Might be useful to catch the original exception and use a raise from with a more user-friendly message.
• Similarly, if scheme is not given in the dict, this leads to a KeyError: 'scheme'. This message should be improved.
• I think we should rename the scheme key in the dict to something else to avoid duplicating the word "scheme". Maybe something like name, module, or method?

  regrid:
    regrid:
      target_grid: 0.5x0.5
      scheme:
        name/module/method: iris.analysis.Linear
        extrapolation_mode: mask

[valeriupredoi]

nice idea, I likes! One quick prelim suggestion from me, and of course, as Lenin used to say - tests, tests and tests again 😁 (lemme know I can help with the tests)

[zklaus]

Alright, thanks @schlunma and @valeriupredoi. I'll take that as an initial go-ahead and will come back with a more fleshed out version.

[schlunma]

@zklaus You think this will be ready in time for v2.5.0? I think it would be nice to include this.

[zklaus]

I just successfully tested this PR with a couple if iris schemes (e.g. iris.analysis.Linear) tada It works really well and just like expected.

Cheers 😄

Three small comments:

* If an invalid scheme is provided, the error message can be really cryptic. Might be useful to catch the original exception and use a `raise from` with a more user-friendly message.

Done. Ok like this?

* Similarly, if `scheme` is not given in the `dict`, this leads to a `KeyError: 'scheme'`. This message should be improved.

Ditto. Ok?

* I think we should rename the `scheme` key in the `dict` to something else to avoid duplicating the word "scheme". Maybe something like `name`, `module`, or `method`?

Agree completely. In fact, I had been thinking about it, but name struck me as not technical enough, module and method are not accurate since it can be any callable that produces a regridding scheme. In playing with the options and looking around a bit, I landed on reference because in the end I followed the syntax for object references from the entry point data model. What do you think?

[zklaus]

Now on to writing some tests.

[schlunma]

Sounds good @zklaus ! You think you can finish this by the end of this week? Would be great to include this in the (hopfully) last rc5, so we can run the recipes with theses changes.

[zklaus]

The main purpose of this PR is to allow the use of iris-esmf-regrid to enable faster, lazy regridding all around and to facilitate regridding of unstructured grids with the new iris mesh support. However, the mechanism introduced here is more general, and hence I introduce neither iris-esmf-regrid as a dependency, nor extensive specific documentation. For documentation we rely on the documentation of iris-esmf-regrid itself; as a dependency this should be introduced alongside the recipe that makes use of it.

Of course, this can be debated. If you prefer to integrate things more tightly here, we can take this now, or in a later PR for the next version. That would still allow us to do more experimentation with the released core in the next development cycle of the tool.

[schlunma]

Great, thanks Klaus!

I think it would have been nice to already include the iris-esmf-regrid dependency with this PR, but I'm also fine with just adding this framework. Please make sure to open an issue about this (with the label v2.6) so we don't forget about this.

Will have a look at this PR now!

[schlunma]

One further thought about this after reading the doc: I noticed that you use iris-esmf-regrid quite often in the doc. If iris-esmf-regrid is mentioned there, I think we need to include this.

Would it make sense to include it as a dependency, add a very small test, e.g.,

def test_regrid_generic_regridding(self):
    regrid(self.src_cube, '1x1', {"reference": "esmf_regrid.schemes:ESMFAreaWeighted"})

and add a note to the doc that this feature is still "experimental"?

[zklaus]

Actually forget my last comment (that I have now hidden). The package is there. I'll investigate. I will say that I got a response of "currently the PyPI servers are overloaded" when trying pip search esmf_regrid locally before, but I am not completely sure that was the problem for the CI as well.

[valeriupredoi]

can't find it, man - use the web based search https://pypi.org/search/?q=iris-esmf-regrid

[valeriupredoi]

this one maybe? Says it's the bridge between Iris and ESMF (A Bridge Too Far, if you asked me 😆 ) https://pypi.org/project/esmf-regrid/

[zklaus]

Yeah, of course that's the one. As named in setup.py. But what are you trying to say?

[valeriupredoi]

nothing, I basically just landed in this discussion and still need to figure my way around - maybe I should not comment until then 😆

[zklaus]

Hm. Seems iris-esmf-regrid limits itself to Python 3.8 at the moment.

[schlunma]

Ahh...well that's bad. I don't think there is an easy way to solve this apart from dropping support for python 3.7, right?

In that case it's probably best to remove it from the environment again (and also remove the test) and explicitly mention in the doc that it needs to be installed manually and requires python 3.8.

[valeriupredoi]

Klaus opened an issue here SciTools/iris-esmf-regrid#161 - I've been snooping around (mostly looking at their conda lock file) and it's not obvious to me at all why Python<3.9 - all deps seem to be at their latest versions but built specifically against Python=3.8

[schlunma]

Ahh, it's python=3.8, not python>=3.8, misunderstood that 😬

[valeriupredoi]

🐍 < 3️⃣ 9️⃣ Manu

[zklaus]

I think in any case it is reasonable to remove it from the environment. After all, it really is only an example (from the point of view of the generic regridder).

[schlunma]

Agreed, but then we need to make it crystal clear in the documentation that iris-esmf-regrid needs to be installed manually and that only python<3.8 is supported.

[zklaus]

I have removed the dependency and improved the tests.

I am not so sure we need to be very forceful about how to install iris-esmf-regrid.

First, it really is only an example. If you want to use a different regridder or advanced features of the Iris regridders (perhaps some that will be added in the future), this works just as well, so I don't want to focus too much on this particular package.
Second, practically, it will not be much of an issue. Most people will not insist on a Python version and just ask for things to be installed, which will work out fine. We run into the problem in the CI because we test all versions.
Third, it is likely to change and since we also state that it is experimental, people should not be surprised with hiccups like this.

So imho it's better to have simple, accessible documentation and to refer to the iris-esmf-regrid documentation for further details.

Now on to writing the extended documentation requested by @schlunma.

[zklaus]

Done. One last round of reviews, @schlunma? @valeriupredoi do you want to have a look too and see if your blocking change request is sufficiently addressed?

[valeriupredoi]

cheers, Klaus! I'll uncork meself in a minute 👍

[valeriupredoi]

very nice, great work, Klaus! Juat a couple typos from me, all good 🍻

[schlunma]

Cheers Klaus, very nice work!

I tested this with actual data and it seems to work fine.

One very tiny comment on the doc.