Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions doc/recipe/preprocessor.rst
Original file line number Diff line number Diff line change
Expand Up @@ -926,10 +926,6 @@ example of its usage in an ESMValTool preprocessor is:
reference: esmf_regrid.schemes:ESMFAreaWeighted
mdtol: 0.7

.. TODO: Remove the following warning once things have settled a bit.
.. warning::
Just as the mesh support in Iris itself, this new regridding package is
still considered experimental.

.. _ensemble statistics:

Expand Down
1 change: 1 addition & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ dependencies:
- humanfriendly
- importlib_resources
- iris>=3.2.1
- iris-esmf-regrid
- isodate
- jinja2
- nc-time-axis
Expand Down
27 changes: 17 additions & 10 deletions esmvalcore/preprocessor/_regrid.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Horizontal and vertical regridding module."""

import importlib
import inspect
import logging
import os
import re
Expand Down Expand Up @@ -536,8 +537,7 @@ def regrid(cube, target_grid, scheme, lat_offset=True, lon_offset=True):
extrapolation_mode: nanmask

To use the area weighted regridder available in
:class:`esmf_regrid.schemes.ESMFAreaWeighted`, make sure that
:doc:`iris-esmf-regrid:index` is installed and use
:class:`esmf_regrid.schemes.ESMFAreaWeighted` use

.. code-block:: yaml

Expand All @@ -547,10 +547,8 @@ def regrid(cube, target_grid, scheme, lat_offset=True, lon_offset=True):
scheme:
reference: esmf_regrid.schemes:ESMFAreaWeighted

.. note::

Note that :doc:`iris-esmf-regrid:index` is still experimental.
"""
scheme_args = None
if isinstance(scheme, dict):
try:
object_ref = scheme.pop("reference")
Expand All @@ -568,12 +566,12 @@ def regrid(cube, target_grid, scheme, lat_offset=True, lon_offset=True):
if separator:
for attr in scheme_name.split('.'):
obj = getattr(obj, attr)
loaded_scheme = obj(**scheme)
scheme_args = inspect.getfullargspec(obj).args
else:
loaded_scheme = HORIZONTAL_SCHEMES.get(scheme.lower())
if loaded_scheme is None:
emsg = 'Unknown regridding scheme, got {!r}.'
raise ValueError(emsg.format(scheme))
if loaded_scheme is None:
emsg = 'Unknown regridding scheme, got {!r}.'
raise ValueError(emsg.format(scheme))

if isinstance(target_grid, str):
if os.path.isfile(target_grid):
Expand All @@ -600,6 +598,13 @@ def regrid(cube, target_grid, scheme, lat_offset=True, lon_offset=True):
if not isinstance(target_grid, iris.cube.Cube):
raise ValueError('Expecting a cube, got {}.'.format(target_grid))

if isinstance(scheme_args, list):
if 'src_cube' in scheme_args:
scheme['src_cube'] = cube
if 'grid_cube' in scheme_args:
scheme['grid_cube'] = target_grid
loaded_scheme = obj(**scheme)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to add this block to l.569. By doing this you will end up with a loaded_scheme for both the if and else statement. You could also restore the lines 574-576 then.

Can scheme_args be something else than None or a list? If not, I think

if scheme_args is not None:

might be a little bit clearer. Also, can you add some comments what you do here? That would really help to understand the code better. E.g., why are you checking the args of obj, why are you extending them with the cube and tgrid, etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I thought it would fit better in those lines as well. However, there two schemes in iris-esmf.

The current main, supports the call to esmf_regrid.schemes.ESMFAreaWeighted(mdtol=0), which returns a scheme to be called by cube.regrid.

But esmf_regrid.schemes.regrid_rectilinear_to_rectilinear does not return a scheme, it's the regridding routine on it's own. It expects cubes as inputs and returns a regridded cube as output. And at that point the target cube has not been generated yet.

That is why the arguments are being checked. Because one scheme does not require them (unless we want to allow mdtol to be set as well, which is not possible right now), whereas another one fails if the src_cube and grid_cube are not given as inputs.

Would it work for you if the routine checked first the if isinstance(target_grid, str) block and then the if isinstance(scheme, dict) one? At first sight it does not look like it would break anything and it would be more organised.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, you are totally right, I didn't notice that target_grid is defined only later! I think swapping the blocks if isinstance(target_grid, str) and if isinstance(scheme, dict) is a good idea!

About the mdtol: I think it currently is possible to set this with

scheme:
  reference: esmf_regrid.schemes.ESMFAreaWeighted
  mdtol: 0.5

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah true, then essentially the only tricky arguments were src_cube and grid_cube, since those cannot be passed in the recipe. I have updated the code with these changes

# Unstructured regridding requires x2 2d spatial coordinates,
# so ensure to purge any 1d native spatial dimension coordinates
# for the regridder.
Expand All @@ -625,11 +630,13 @@ def regrid(cube, target_grid, scheme, lat_offset=True, lon_offset=True):
else:
fill_value = GLOBAL_FILL_VALUE
da.ma.set_fill_value(cube.core_data(), fill_value)

# Perform the horizontal regridding
if _attempt_irregular_regridding(cube, scheme):
cube = esmpy_regrid(cube, target_grid, scheme)
else:
if isinstance(loaded_scheme, iris.cube.Cube):
return loaded_scheme

cube = cube.regrid(target_grid, loaded_scheme)
Comment on lines 634 to 642

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if _attempt_irregular_regridding(cube, scheme):
cube = esmpy_regrid(cube, target_grid, scheme)
else:
if isinstance(loaded_scheme, iris.cube.Cube):
return loaded_scheme
cube = cube.regrid(target_grid, loaded_scheme)
if _attempt_irregular_regridding(cube, scheme):
cube = esmpy_regrid(cube, target_grid, scheme)
elif isinstance(loaded_scheme, iris.cube.Cube):
cube = loaded_scheme
else:
cube = cube.regrid(target_grid, loaded_scheme)

Maybe clearer? Could you also add a comment here in which cases loaded_scheme is a Cube?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I have also updated the documentation.


# Preserve dtype and use masked arrays for 'unstructured_nearest'
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
'cf-units',
'dask[array]',
'esgf-pyclient>=0.3.1',
'esmf-regrid',
'esmpy!=8.1.0', # see github.com/ESMValGroup/ESMValCore/issues/1208
'fiona',
'fire',
Expand Down
33 changes: 33 additions & 0 deletions tests/integration/preprocessor/_regrid/test_regrid.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ def test_regrid__linear(self):
expected = np.array([[[1.5]], [[5.5]], [[9.5]]])
self.assert_array_equal(result.data, expected)

def test_regrid__esmf_rectilinear(self):
scheme_name = 'esmf_regrid.schemes:regrid_rectilinear_to_rectilinear'
scheme = {
'reference': scheme_name
}
result = regrid(
self.cube,
self.grid_for_linear,
scheme)
expected = np.array([[[1.5]], [[5.5]], [[9.5]]])
np.testing.assert_array_almost_equal(result.data, expected, decimal=1)

def test_regrid__regular_coordinates(self):
data = np.ones((1, 1))
lons = iris.coords.DimCoord([1.50000000000001],
Expand Down Expand Up @@ -214,6 +226,27 @@ def test_regrid__area_weighted(self):
expected = np.array([1.499886, 5.499886, 9.499886])
np.testing.assert_array_almost_equal(result.data, expected, decimal=6)

def test_regrid__esmf_area_weighted(self):
data = np.empty((1, 1))
lons = iris.coords.DimCoord([1.6],
standard_name='longitude',
bounds=[[1, 2]],
units='degrees_east',
coord_system=self.cs)
lats = iris.coords.DimCoord([1.6],
standard_name='latitude',
bounds=[[1, 2]],
units='degrees_north',
coord_system=self.cs)
coords_spec = [(lats, 0), (lons, 1)]
grid = iris.cube.Cube(data, dim_coords_and_dims=coords_spec)
scheme = {
'reference': 'esmf_regrid.schemes:ESMFAreaWeighted'
}
result = regrid(self.cube, grid, scheme)
expected = np.array([[[1.499886]], [[5.499886]], [[9.499886]]])
np.testing.assert_array_almost_equal(result.data, expected, decimal=6)

def test_regrid__unstructured_nearest_float(self):
"""Test unstructured_nearest regridding with cube of floats."""
result = regrid(self.unstructured_grid_cube,
Expand Down