From c299d1ba03193c9a52f1b879e8449b325f852b8e Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Wed, 3 Nov 2021 12:11:57 +0100 Subject: [PATCH 1/3] Removed custom version of atmosphere sigma coordinate factory --- esmvalcore/cmor/_fixes/cmip5/fgoals_g2.py | 25 +-- esmvalcore/cmor/_fixes/shared.py | 175 ------------------ esmvalcore/cmor/fixes.py | 7 +- esmvalcore/preprocessor/_io.py | 7 +- .../cmor/_fixes/cmip5/test_fgoals_g2.py | 24 +-- tests/integration/cmor/_fixes/test_shared.py | 40 +--- tests/unit/cmor/test_cmor_check.py | 1 + tests/unit/cmor/test_fixes.py | 1 - 8 files changed, 11 insertions(+), 269 deletions(-) diff --git a/esmvalcore/cmor/_fixes/cmip5/fgoals_g2.py b/esmvalcore/cmor/_fixes/cmip5/fgoals_g2.py index 5d5cbcda23..1c2df64db0 100644 --- a/esmvalcore/cmor/_fixes/cmip5/fgoals_g2.py +++ b/esmvalcore/cmor/_fixes/cmip5/fgoals_g2.py @@ -3,7 +3,7 @@ from cf_units import Unit from ..fix import Fix -from ..shared import add_sigma_factory, round_coordinates +from ..shared import round_coordinates class AllVars(Fix): @@ -36,26 +36,3 @@ def fix_metadata(self, cubes): round_coordinates(cubes, 4, coord_names=['longitude']) return cubes - - -class Cl(Fix): - """Fixes for ``cl``.""" - - def fix_metadata(self, cubes): - """Fix metadata. - - Add pressure level coordinate. - - Parameters - ---------- - cubes : iris.cube.CubeList - Input cubes. - - Returns - ------- - iris.cube.CubeList - - """ - cube = self.get_cube_from_list(cubes) - add_sigma_factory(cube) - return iris.cube.CubeList([cube]) diff --git a/esmvalcore/cmor/_fixes/shared.py b/esmvalcore/cmor/_fixes/shared.py index f013daca8b..f6f4400dcd 100644 --- a/esmvalcore/cmor/_fixes/shared.py +++ b/esmvalcore/cmor/_fixes/shared.py @@ -1,7 +1,6 @@ """Shared functions for fixes.""" import logging import os -import warnings from functools import lru_cache import dask.array as da @@ -15,180 +14,6 @@ logger = logging.getLogger(__name__) -class AtmosphereSigmaFactory(iris.aux_factory.AuxCoordFactory): - """Defines an atmosphere sigma coordinate factory.""" - - def __init__(self, pressure_at_top=None, sigma=None, - surface_air_pressure=None): - """Create class instance. - - Creates and atmosphere sigma coordinate factory with the formula: - - p(n, k, j, i) = pressure_at_top + sigma(k) * - (surface_air_pressure(n, j, i) - pressure_at_top) - """ - self._metadata_manager = iris.common.metadata_manager_factory( - iris.common.CoordMetadata) - super().__init__() - self._check_dependencies(pressure_at_top, sigma, surface_air_pressure) - self.units = pressure_at_top.units - self.pressure_at_top = pressure_at_top - self.sigma = sigma - self.surface_air_pressure = surface_air_pressure - self.standard_name = 'air_pressure' - self.attributes = {} - - @staticmethod - def _check_dependencies(pressure_at_top, sigma, surface_air_pressure): - """Check for sufficient coordinates.""" - if any([ - pressure_at_top is None, - sigma is None, - surface_air_pressure is None, - ]): - raise ValueError( - "Unable to construct atmosphere sigma coordinate factory due " - "to insufficient source coordinates") - - # Check dimensions - if pressure_at_top.shape not in ((), (1, )): - raise ValueError( - f"Expected scalar 'pressure_at_top' coordinate, got shape " - f"{pressure_at_top.shape}") - - # Check bounds - if sigma.nbounds not in (0, 2): - raise ValueError( - f"Invalid 'sigma' coordinate: must have either 0 or 2 bounds, " - f"got {sigma.nbounds:d}") - for coord in (pressure_at_top, surface_air_pressure): - if coord.nbounds: - msg = (f"Coordinate '{coord.name()}' has bounds. These will " - "be disregarded") - warnings.warn(msg, UserWarning, stacklevel=2) - - # Check units - if sigma.units.is_unknown(): - sigma.units = Unit('1') - if not sigma.units.is_dimensionless(): - raise ValueError( - f"Invalid units: 'sigma' must be dimensionless, got " - f"'{sigma.units}'") - if pressure_at_top.units != surface_air_pressure.units: - raise ValueError( - f"Incompatible units: 'pressure_at_top' and " - f"'surface_air_pressure' must have the same units, got " - f"'{pressure_at_top.units}' and " - f"'{surface_air_pressure.units}'") - if not pressure_at_top.units.is_convertible('Pa'): - raise ValueError( - "Invalid units: 'pressure_at_top' and 'surface_air_pressure' " - "must have units of pressure") - - @property - def dependencies(self): - """Return dependencies.""" - dependencies = { - 'pressure_at_top': self.pressure_at_top, - 'sigma': self.sigma, - 'surface_air_pressure': self.surface_air_pressure, - } - return dependencies - - @staticmethod - def _derive(pressure_at_top, sigma, surface_air_pressure): - """Derive coordinate.""" - return pressure_at_top + sigma * (surface_air_pressure - - pressure_at_top) - - def make_coord(self, coord_dims_func): - """Make new :class:`iris.coords.AuxCoord`.""" - # Which dimensions are relevant? - derived_dims = self.derived_dims(coord_dims_func) - dependency_dims = self._dependency_dims(coord_dims_func) - - # Build the points array - nd_points_by_key = self._remap(dependency_dims, derived_dims) - points = self._derive(nd_points_by_key['pressure_at_top'], - nd_points_by_key['sigma'], - nd_points_by_key['surface_air_pressure']) - - # Bounds - bounds = None - if self.sigma.nbounds: - nd_values_by_key = self._remap_with_bounds(dependency_dims, - derived_dims) - pressure_at_top = nd_values_by_key['pressure_at_top'] - sigma = nd_values_by_key['sigma'] - surface_air_pressure = nd_values_by_key['surface_air_pressure'] - ok_bound_shapes = [(), (1,), (2,)] - if sigma.shape[-1:] not in ok_bound_shapes: - raise ValueError("Invalid sigma coordinate bounds") - if pressure_at_top.shape[-1:] not in [(), (1,)]: - warnings.warn( - "Pressure at top coordinate has bounds. These are being " - "disregarded") - pressure_at_top_pts = nd_points_by_key['pressure_at_top'] - bds_shape = list(pressure_at_top_pts.shape) + [1] - pressure_at_top = pressure_at_top_pts.reshape(bds_shape) - if surface_air_pressure.shape[-1:] not in [(), (1,)]: - warnings.warn( - "Surface pressure coordinate has bounds. These are being " - "disregarded") - surface_air_pressure_pts = nd_points_by_key[ - 'surface_air_pressure'] - bds_shape = list(surface_air_pressure_pts.shape) + [1] - surface_air_pressure = surface_air_pressure_pts.reshape( - bds_shape) - bounds = self._derive(pressure_at_top, sigma, surface_air_pressure) - - # Create coordinate - return iris.coords.AuxCoord( - points, standard_name=self.standard_name, long_name=self.long_name, - var_name=self.var_name, units=self.units, bounds=bounds, - attributes=self.attributes, coord_system=self.coord_system) - - def update(self, old_coord, new_coord=None): - """Notify the factory of the removal/replacement of a coordinate.""" - new_dependencies = self.dependencies - for (name, coord) in self.dependencies.items(): - if old_coord is coord: - new_dependencies[name] = new_coord - try: - self._check_dependencies(**new_dependencies) - except ValueError as exc: - raise ValueError(f"Failed to update dependencies: {exc}") - else: - setattr(self, name, new_coord) - break - - -def add_sigma_factory(cube): - """Add factory for ``atmosphere_sigma_coordinate``. - - Parameters - ---------- - cube : iris.cube.Cube - Input cube. - - Raises - ------ - ValueError - ``cube`` does not contain coordinate ``atmosphere_sigma_coordinate``. - """ - if cube.coords('atmosphere_sigma_coordinate'): - aux_factory = AtmosphereSigmaFactory( - pressure_at_top=cube.coord(var_name='ptop'), - sigma=cube.coord(var_name='lev'), - surface_air_pressure=cube.coord(var_name='ps'), - ) - cube.add_aux_factory(aux_factory) - return - raise ValueError( - "Cannot add 'air_pressure' coordinate, 'atmosphere_sigma_coordinate' " - "coordinate not available") - - def add_aux_coords_from_cubes(cube, cubes, coord_dict): """Add auxiliary coordinate to cube from another cube in list of cubes. diff --git a/esmvalcore/cmor/fixes.py b/esmvalcore/cmor/fixes.py index 5827dffc48..e5931b0f0f 100644 --- a/esmvalcore/cmor/fixes.py +++ b/esmvalcore/cmor/fixes.py @@ -1,13 +1,8 @@ """Functions for fixing specific issues with datasets.""" -from ._fixes.shared import ( - add_altitude_from_plev, - add_plev_from_altitude, - add_sigma_factory, -) +from ._fixes.shared import add_altitude_from_plev, add_plev_from_altitude __all__ = [ 'add_altitude_from_plev', 'add_plev_from_altitude', - 'add_sigma_factory', ] diff --git a/esmvalcore/preprocessor/_io.py b/esmvalcore/preprocessor/_io.py index 6ebb9a2665..83cacef5ce 100644 --- a/esmvalcore/preprocessor/_io.py +++ b/esmvalcore/preprocessor/_io.py @@ -7,12 +7,12 @@ from warnings import catch_warnings, filterwarnings import iris +import iris.aux_factory import iris.exceptions import numpy as np import yaml from .._task import write_ncl_settings -from ..cmor._fixes.shared import AtmosphereSigmaFactory from ._time import extract_time logger = logging.getLogger(__name__) @@ -64,13 +64,14 @@ def _fix_aux_factories(cube): # Atmosphere sigma coordinate if 'atmosphere_sigma_coordinate' in coord_names: - new_aux_factory = AtmosphereSigmaFactory( + new_aux_factory = iris.aux_factory.AtmosphereSigmaFactory( pressure_at_top=cube.coord(var_name='ptop'), sigma=cube.coord(var_name='lev'), surface_air_pressure=cube.coord(var_name='ps'), ) for aux_factory in cube.aux_factories: - if isinstance(aux_factory, AtmosphereSigmaFactory): + if isinstance(aux_factory, + iris.aux_factory.AtmosphereSigmaFactory): break else: cube.add_aux_factory(new_aux_factory) diff --git a/tests/integration/cmor/_fixes/cmip5/test_fgoals_g2.py b/tests/integration/cmor/_fixes/cmip5/test_fgoals_g2.py index 0169bba5d7..2cfffcdb3f 100644 --- a/tests/integration/cmor/_fixes/cmip5/test_fgoals_g2.py +++ b/tests/integration/cmor/_fixes/cmip5/test_fgoals_g2.py @@ -3,11 +3,10 @@ from cf_units import Unit from iris.coords import DimCoord -from iris.cube import Cube, CubeList +from iris.cube import Cube -from esmvalcore.cmor._fixes.cmip5.fgoals_g2 import AllVars, Cl +from esmvalcore.cmor._fixes.cmip5.fgoals_g2 import AllVars from esmvalcore.cmor.fix import Fix -from esmvalcore.cmor.table import get_var_info class TestAll(unittest.TestCase): @@ -54,22 +53,3 @@ def test_fix_metadata_dont_fail_if_not_time(self): """Test calendar fix.""" self.cube.remove_coord('time') self.fix.fix_metadata([self.cube]) - - -def test_get_cl_fix(): - """Test getting of fix.""" - fix = Fix.get_fixes('CMIP5', 'FGOALS-g2', 'Amon', 'cl') - assert fix == [Cl(None), AllVars(None)] - - -@unittest.mock.patch( - 'esmvalcore.cmor._fixes.cmip5.fgoals_g2.add_sigma_factory', - autospec=True) -def test_cl_fix_metadata(mock_add_sigma_factory): - """Test ``fix_metadata`` for ``cl``.""" - cubes = CubeList([Cube(0.0, var_name='cl'), Cube(1.0, var_name='x')]) - vardef = get_var_info('CMIP6', 'Amon', 'cl') - fix = Cl(vardef) - fixed_cubes = fix.fix_metadata(cubes) - mock_add_sigma_factory.assert_called_once_with(cubes[0]) - assert fixed_cubes == CubeList([cubes[0]]) diff --git a/tests/integration/cmor/_fixes/test_shared.py b/tests/integration/cmor/_fixes/test_shared.py index 69e9a0f092..741c5e4233 100644 --- a/tests/integration/cmor/_fixes/test_shared.py +++ b/tests/integration/cmor/_fixes/test_shared.py @@ -1,5 +1,7 @@ """Tests for shared functions for fixes.""" import iris +import iris.coords +import iris.cube import numpy as np import pytest from cf_units import Unit @@ -13,7 +15,6 @@ add_scalar_typeland_coord, add_scalar_typesea_coord, add_scalar_typesi_coord, - add_sigma_factory, cube_to_aux_coord, fix_bounds, fix_ocean_depth_coord, @@ -355,43 +356,6 @@ def test_add_scalar_typesi_coord(cube_in, typesi): assert coord == typesi_coord -PS_COORD = iris.coords.AuxCoord([[[101000.0]]], var_name='ps', units='Pa') -PTOP_COORD = iris.coords.AuxCoord(1000.0, var_name='ptop', units='Pa') -LEV_COORD = iris.coords.AuxCoord([0.5], bounds=[[0.2, 0.8]], var_name='lev', - units='1', - standard_name='atmosphere_sigma_coordinate') -P_COORD_HYBRID = iris.coords.AuxCoord([[[[51000.0]]]], - bounds=[[[[[21000.0, 81000.0]]]]], - standard_name='air_pressure', units='Pa') -CUBE_HYBRID = iris.cube.Cube([[[[1.0]]]], var_name='x', - aux_coords_and_dims=[(PS_COORD, (0, 2, 3)), - (PTOP_COORD, ()), - (LEV_COORD, 1)]) - - -TEST_ADD_SIGMA_FACTORY = [ - (CUBE_HYBRID.copy(), P_COORD_HYBRID.copy()), - (iris.cube.Cube(0.0), None), -] - - -@pytest.mark.sequential -@pytest.mark.parametrize('cube,output', TEST_ADD_SIGMA_FACTORY) -def test_add_sigma_factory(cube, output): - """Test adding of factory for ``atmosphere_sigma_coordinate``.""" - if output is None: - with pytest.raises(ValueError) as err: - add_sigma_factory(cube) - msg = ("Cannot add 'air_pressure' coordinate, " - "'atmosphere_sigma_coordinate' coordinate not available") - assert str(err.value) == msg - return - assert not cube.coords('air_pressure') - add_sigma_factory(cube) - air_pressure_coord = cube.coord('air_pressure') - assert air_pressure_coord == output - - @pytest.mark.sequential def test_cube_to_aux_coord(): """Test converting cube to auxiliary coordinate.""" diff --git a/tests/unit/cmor/test_cmor_check.py b/tests/unit/cmor/test_cmor_check.py index e8736b1e57..cc5ebdac2d 100644 --- a/tests/unit/cmor/test_cmor_check.py +++ b/tests/unit/cmor/test_cmor_check.py @@ -6,6 +6,7 @@ import iris import iris.coord_categorisation import iris.coords +import iris.cube import iris.util import numpy as np from cf_units import Unit diff --git a/tests/unit/cmor/test_fixes.py b/tests/unit/cmor/test_fixes.py index 32ae18e5ae..24e4acde52 100644 --- a/tests/unit/cmor/test_fixes.py +++ b/tests/unit/cmor/test_fixes.py @@ -8,7 +8,6 @@ @pytest.mark.parametrize('func', [ 'add_altitude_from_plev', 'add_plev_from_altitude', - 'add_sigma_factory', ]) def test_imports(func): assert func in fixes.__all__ From f6d8094856824da90f1c4fac1ae4ea8dd9e3f082 Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Wed, 3 Nov 2021 12:36:47 +0100 Subject: [PATCH 2/3] Fixed CMIP6 fix --- esmvalcore/cmor/_fixes/cmip6/fgoals_g3.py | 7 ---- .../cmor/_fixes/cmip6/test_fgoals_g3.py | 37 +------------------ 2 files changed, 1 insertion(+), 43 deletions(-) diff --git a/esmvalcore/cmor/_fixes/cmip6/fgoals_g3.py b/esmvalcore/cmor/_fixes/cmip6/fgoals_g3.py index 064a75e5e7..4126ef524b 100644 --- a/esmvalcore/cmor/_fixes/cmip6/fgoals_g3.py +++ b/esmvalcore/cmor/_fixes/cmip6/fgoals_g3.py @@ -1,16 +1,9 @@ """Fixes for FGOALS-g3 model.""" import iris -from ..cmip5.fgoals_g2 import Cl as BaseCl from ..common import OceanFixGrid from ..fix import Fix -Cl = BaseCl - -Cli = BaseCl - -Clw = BaseCl - def _check_bounds_monotonicity(coord): """Check monotonicity of a coords bounds array.""" diff --git a/tests/integration/cmor/_fixes/cmip6/test_fgoals_g3.py b/tests/integration/cmor/_fixes/cmip6/test_fgoals_g3.py index da847f0037..630aa99fca 100644 --- a/tests/integration/cmor/_fixes/cmip6/test_fgoals_g3.py +++ b/tests/integration/cmor/_fixes/cmip6/test_fgoals_g3.py @@ -4,47 +4,12 @@ import iris import numpy as np -from esmvalcore.cmor._fixes.cmip5.fgoals_g2 import Cl as BaseCl -from esmvalcore.cmor._fixes.cmip6.fgoals_g3 import Cl, Cli, Clw, Siconc -from esmvalcore.cmor._fixes.cmip6.fgoals_g3 import Tos, Mrsos +from esmvalcore.cmor._fixes.cmip6.fgoals_g3 import Mrsos, Siconc, Tos from esmvalcore.cmor._fixes.common import OceanFixGrid from esmvalcore.cmor.fix import Fix from esmvalcore.cmor.table import get_var_info -def test_get_cl_fix(): - """Test getting of fix.""" - fix = Fix.get_fixes('CMIP6', 'FGOALS-g3', 'Amon', 'cl') - assert fix == [Cl(None)] - - -def test_cl_fix(): - """Test fix for ``cl``.""" - assert Cl is BaseCl - - -def test_get_cli_fix(): - """Test getting of fix.""" - fix = Fix.get_fixes('CMIP6', 'FGOALS-g3', 'Amon', 'cli') - assert fix == [Cli(None)] - - -def test_cli_fix(): - """Test fix for ``cli``.""" - assert Cli is BaseCl - - -def test_get_clw_fix(): - """Test getting of fix.""" - fix = Fix.get_fixes('CMIP6', 'FGOALS-g3', 'Amon', 'clw') - assert fix == [Clw(None)] - - -def test_clw_fix(): - """Test fix for ``clw``.""" - assert Clw is BaseCl - - def test_get_tos_fix(): """Test getting of fix.""" fix = Fix.get_fixes('CMIP6', 'BCC-CSM2-MR', 'Omon', 'tos') From 448435c0ab55089f12636ab5bee4c0f49242c50f Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Wed, 3 Nov 2021 13:00:59 +0100 Subject: [PATCH 3/3] Added missing tests for concatenation --- .../preprocessor/_io/test_concatenate.py | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tests/integration/preprocessor/_io/test_concatenate.py b/tests/integration/preprocessor/_io/test_concatenate.py index 40722ff1e3..681463f21b 100644 --- a/tests/integration/preprocessor/_io/test_concatenate.py +++ b/tests/integration/preprocessor/_io/test_concatenate.py @@ -7,7 +7,11 @@ import numpy as np import pytest from cf_units import Unit -from iris.aux_factory import HybridHeightFactory, HybridPressureFactory +from iris.aux_factory import ( + AtmosphereSigmaFactory, + HybridHeightFactory, + HybridPressureFactory, +) from iris.coords import AuxCoord, DimCoord from iris.cube import Cube, CubeList from iris.exceptions import ConcatenateError @@ -65,6 +69,31 @@ def mock_empty_cube(): return cube +@pytest.fixture +def mock_atmosphere_sigma_cube(): + """Return mocked cube with atmosphere sigma coordinate.""" + cube = unittest.mock.create_autospec(Cube, spec_set=True, instance=True) + ptop_coord = AuxCoord([1.0], var_name='ptop', units='Pa') + lev_coord = AuxCoord([0.0], bounds=[[-0.5, 1.5]], var_name='lev', + units='1') + ps_coord = AuxCoord([[[100000]]], var_name='ps', units='Pa') + cube.coord.side_effect = [ptop_coord, lev_coord, ps_coord, + ptop_coord, lev_coord, ps_coord] + cube.coords.return_value = [ + ptop_coord, + lev_coord, + ps_coord, + AuxCoord(0.0, standard_name='atmosphere_sigma_coordinate'), + ] + aux_factory = AtmosphereSigmaFactory( + pressure_at_top=ptop_coord, + sigma=lev_coord, + surface_air_pressure=ps_coord, + ) + cube.aux_factories = ['dummy', aux_factory] + return cube + + @pytest.fixture def mock_hybrid_height_cube(): """Return mocked cube with hybrid height coordinate.""" @@ -147,6 +176,29 @@ def test_fix_aux_factories_empty_cube(mock_empty_cube): assert mock_empty_cube.mock_calls == [call.coords()] +def test_fix_aux_factories_atmosphere_sigma(mock_atmosphere_sigma_cube): + """Test fixing of atmosphere sigma coordinate.""" + check_if_fix_aux_factories_is_necessary() + + # Test with aux_factory object + _io._fix_aux_factories(mock_atmosphere_sigma_cube) + mock_atmosphere_sigma_cube.coords.assert_called_once_with() + mock_atmosphere_sigma_cube.coord.assert_has_calls([call(var_name='ptop'), + call(var_name='lev'), + call(var_name='ps')]) + mock_atmosphere_sigma_cube.add_aux_factory.assert_not_called() + + # Test without aux_factory object + mock_atmosphere_sigma_cube.reset_mock() + mock_atmosphere_sigma_cube.aux_factories = ['dummy'] + _io._fix_aux_factories(mock_atmosphere_sigma_cube) + mock_atmosphere_sigma_cube.coords.assert_called_once_with() + mock_atmosphere_sigma_cube.coord.assert_has_calls([call(var_name='ptop'), + call(var_name='lev'), + call(var_name='ps')]) + mock_atmosphere_sigma_cube.add_aux_factory.assert_called_once() + + def test_fix_aux_factories_hybrid_height(mock_hybrid_height_cube): """Test fixing of hybrid height coordinate.""" check_if_fix_aux_factories_is_necessary()