Refactor regridding #2231

schlunma · 2023-10-12T16:52:28Z

Description

This PR refactors our regridding. The main purpose is to simplify the code so that new schemes can be added more easily, which will be necessary for #2178.

Features

At the moment, the schemes linear, area_weighted and nearest work for regular and irregular grids. However, for unstructured grids, we have to use unstructured_nearest. This PR unifies this behavior so that nearest can also be used for unstructured grids (just like for irregular grids, I added a check for unstructured grids here that automatically selects the correct regridding scheme for unstructured grids).
In addition, this PR unifies all regridding schemes so they can be used the (iris-intended) way within Cube.regrid, just like the regridding schemes provided by iris. These are now located in the new esmvalcore.preprocessor.regridding_schemes module. With this, regridding is as easy as

loaded_scheme = _load_scheme(cube, scheme)
cube = cube.regrid(target_grid_cube, loaded_scheme)

Finally, this PR adds a check of the validity of the specified regridding schemes early in an ESMValTool run, so that the tool won't fail because of a bad scheme halfway through the recipe (potentially after hours of running).

Deprecations

This PR deprecated two regridding schemes, which will be removed with ESMValCore v2.13.0:

unstructured_nearest: Please use the scheme nearest instead. This is an exact replacement for data on unstructured grids. ESMValCore is now able to determine the most suitable regridding scheme based on the input data.
linear_extrapolate: Please use a generic scheme with reference: iris.analysis:Linear and extrapolation_mode: extrapolate instead. This is an exact replacement, e.g.:

preprocessors:
      regrid_preprocessor:
        regrid:
          target_grid: 2.5x2.5
          scheme:
            reference: iris.analysis:Linear
            extrapolation_mode: extrapolate

Related to #2178

Link to documentation:

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~~ no: see deprecations above
🛠 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~~ Remaining Codacy checks about Too few public methods (1/2) (too-few-public-methods) cannot be fixed; it is necessary to use classes here due to the way iris regridding schemes are defined.

To help with the number pull requests:

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

valeriupredoi

hey Manu, no actual changes needed, just a couple questions for now - the review block is more for me since I'm not done going through the code yet (just got to _regrid_esmpy

esmvalcore/iris_helpers.py

esmvalcore/preprocessor/_regrid.py

valeriupredoi

this is a very nice unification PR, Manu, many thanks! I am very happy to see this happening, but I have a few concerns related to the way how things are structured ie private/public access to various classes and methods, and a bit of extra complexity that IMHO we can do without. Nothing major tho 🍺

valeriupredoi · 2023-12-08T15:32:48Z

esmvalcore/preprocessor/_regrid_esmpy.py

+            tgt_cube,
+            method=self._METHOD,
+            mask_threshold=self.mask_threshold,
+        )


OK call me old fashioned but to me this is a bit wrong: public method in a private class is something I don't agree with, especially since the ESMPYScheme name doesn't reflect the fact that an actual regridder instance is built within its bellows. What's wrong with the old generic and public regrid wrapper that the user is exposed to?

I agree with you that this looks too complicated, but this is how iris expects regridding schemes to look like! The scheme class needs a public method regridder(self, src_cube, target_grid) that returns an object of a regridder class. This regridder class needs to have a method def __call__(self, cube) that actually performs the regridding. Here is an example for a regridder in iris.

The advantage of this new syntax is that it's much more generic. All of our regridding schemes are used the same way (within iris.cube.Cube.regrid(). In the old version, some schemes used the iris syntax, some use this regrid function that you mentiond, etc.

indeed - I caught that, and liked it - but here's the thing - aligning ourselves to iris is not something we should aim for. If we find a nice, generic, and robust way of writing code and functionality we should go that way, regardless iris do other things; in this case, it's prob useful since we are in essence wrapping our regridding around iris regridding, so I'll grab the bait 🐟

esmvalcore/preprocessor/regrid_schemes.py

valeriupredoi · 2023-12-08T15:38:12Z

esmvalcore/preprocessor/regrid_schemes.py

+            Regridder instance.
+
+        """
+        return _GenericRegridder(src_cube, tgt_cube, self.func, **self.kwargs)


ah I see why the one above is private! OK - I'd argue you can coalesce that class into this one, and get rid of _GenericRegridder, and this is already public and does all the bits one needs

See comment above about how iris implements regridders. This needs to stay like this.

valeriupredoi

approving this with very many thanks to Manu for the (rather heavy lifting) work, but I'd like a second pair of reviewing eyes, specifically @bouweandela or @zklaus please - Manu could you hold fire (merge, that is) until one of those two fellas have looked at it too, if you please, bud 🍺

bouweandela

Looks very nice improvement @schlunma!

A few comments:

It would be nice to avoid leaking private things into the public API (check the API docs page)
Could you please indicate in the API docs for each scheme if it supports lazy data?

esmvalcore/preprocessor/regrid_schemes.py

esmvalcore/preprocessor/_regrid_esmpy.py

esmvalcore/preprocessor/_regrid.py

schlunma · 2024-01-25T17:31:13Z

Thanks for the review @bouweandela, I think I addressed all your comments 👍

bouweandela · 2024-01-31T16:05:07Z

esmvalcore/preprocessor/_regrid_esmpy.py

+    tgt_cube:
+        Cube defining the target grid.
+    method:
+        Regridding algorithm.


It would be nice to mention the valid method names here

bouweandela

Thanks for these improvements @schlunma!

schlunma · 2024-02-01T15:13:19Z

Thanks for the reviews @bouweandela and @valeriupredoi ! Merging this now since two people approved.

valeriupredoi · 2024-02-01T15:23:46Z

yay 🥳

schlunma and others added 30 commits August 18, 2023 13:56

First working prototype of ERA5 GRIB reader

f998ae3

Extended list of supported variables for ERA5 GRIB support

8c48834

Merge remote-tracking branch 'origin/main' into read_era5_grib

9740cd3

Added public function to check for unstructured grids

60488dc

Make regridding much faster

f9a4ab7

Add support for more variables and make regridding optional

fc7384a

Add doc

e0c4da3

Added first tests

3db12bb

Added test for loading grib files

09aabcb

Added iris-grib to environment and setup.py

8c73373

Fixed environment

744c20b

Fixed eccodes dependency

0c8ce64

Next try to get environment working

39c6677

Temporarily remove GRIB loading test

b7a0c68

Fixed tests

e907ff1

Added missing tests

0b8fbfa

Fixed test

bef5b5e

Improved test coverage of ERA5 CMORizer

2693066

Increased test coverage of regrid module

e7e7285

Optimized doc

911ed28

More customizable automatic regriddind for ERA5 GRIB

7afa551

Merge branch 'main' into read_era5_grib

7d09738

Merge remote-tracking branch 'origin/main' into read_era5_grib

d0ae8d2

Added iris-grib to setup.py

b4972c9

turn on GA tests

6be7059

Merge remote-tracking branch 'origin/main' into read_era5_grib

0a01d90

Removed ERA5-related changes

c4c5b30

Refactor regridding

4d2dd0d

Deprecate unstructured_nearest and linear_extrapolate

d130e28

Removed UnstructuredLinear from this branch

105004b

schlunma and others added 2 commits November 15, 2023 09:58

Merge branch 'main' into refactor_regridding

9219578

Merge branch 'main' into refactor_regridding

7933bda

valeriupredoi requested changes Dec 8, 2023

View reviewed changes

esmvalcore/iris_helpers.py Show resolved Hide resolved

esmvalcore/preprocessor/_regrid.py Outdated Show resolved Hide resolved

esmvalcore/preprocessor/_regrid.py Show resolved Hide resolved

valeriupredoi requested changes Dec 8, 2023

View reviewed changes

schlunma added 2 commits December 11, 2023 14:21

Merge remote-tracking branch 'origin/main' into refactor_regridding

8da224a

Added public function to check for irregular grid

13f7499

valeriupredoi approved these changes Dec 13, 2023

View reviewed changes

schlunma mentioned this pull request Dec 15, 2023

Add support for native ERA5 data in GRIB format #2178

Merged

15 tasks

Merge remote-tracking branch 'origin/main' into refactor_regridding

92eb068

bouweandela reviewed Jan 25, 2024

View reviewed changes

esmvalcore/preprocessor/regrid_schemes.py Show resolved Hide resolved

esmvalcore/preprocessor/_regrid_esmpy.py Show resolved Hide resolved

esmvalcore/preprocessor/_regrid.py Outdated Show resolved Hide resolved

esmvalcore/preprocessor/_regrid.py Outdated Show resolved Hide resolved

schlunma added 8 commits January 25, 2024 17:17

Merge remote-tracking branch 'origin/main' into refactor_regridding

979fdcf

Avoid leaking private API into docs

2beb1d3

Fix tests

7ce5243

Use functools.lru_cache

969084d

Use core_points() and core_bounds()

cdb1f47

Expanded docstring

9d32aed

Removed deprecated np.alltrue

71371ae

Added regridder classes to public API

15e02cb

bouweandela reviewed Jan 31, 2024

View reviewed changes

bouweandela approved these changes Jan 31, 2024

View reviewed changes

schlunma added 2 commits February 1, 2024 15:57

Mention allowed methods for ESMPyRegridder

c25bef7

Merge remote-tracking branch 'origin/main' into refactor_regridding

ee9503a

schlunma merged commit b0d30f6 into main Feb 1, 2024

schlunma deleted the refactor_regridding branch February 1, 2024 15:13

schlunma mentioned this pull request Feb 16, 2024

Allow deprecated regridding scheme unstructured_nearest in recipe checks #2336

Merged

9 tasks

bouweandela mentioned this pull request Jun 26, 2024

Update the version number and release notes for v2.11.0rc2 #2462

Merged

5 tasks

schlunma mentioned this pull request Jun 2, 2025

Remove deprecated horizontal regridding schemes unstructured_nearest and linear_extrapolate #2743

Merged

9 tasks

Refactor regridding #2231

Refactor regridding #2231

Uh oh!

Conversation

schlunma commented Oct 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Features

Deprecations

Uh oh!

valeriupredoi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

valeriupredoi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valeriupredoi left a comment

Choose a reason for hiding this comment

Uh oh!

bouweandela left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

schlunma commented Jan 25, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bouweandela left a comment

Choose a reason for hiding this comment

Uh oh!

schlunma commented Feb 1, 2024

Uh oh!

valeriupredoi commented Feb 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

schlunma commented Oct 12, 2023 •

edited

Loading