From a8c2d18437a1386b85adcf0c8bbb9d1c21e14ea1 Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Wed, 19 Aug 2020 15:19:38 +0200 Subject: [PATCH 01/10] Added check for units at the end of derivation function --- esmvalcore/preprocessor/_derive/__init__.py | 17 +- .../preprocessor/_derive/test_interface.py | 148 +++++++++++++++++- 2 files changed, 159 insertions(+), 6 deletions(-) diff --git a/esmvalcore/preprocessor/_derive/__init__.py b/esmvalcore/preprocessor/_derive/__init__.py index db4d2c169c..17209f0006 100644 --- a/esmvalcore/preprocessor/_derive/__init__.py +++ b/esmvalcore/preprocessor/_derive/__init__.py @@ -103,9 +103,24 @@ def derive(cubes, short_name, long_name, units, standard_name=None): cube.var_name = short_name cube.standard_name = standard_name if standard_name else None cube.long_name = long_name - cube.units = units for temp in cubes: if 'source_file' in temp.attributes: cube.attributes['source_file'] = temp.attributes['source_file'] + # Check/convert units + if cube.units is None or cube.units == units: + cube.units = units + elif cube.units.is_no_unit() or cube.units.is_unknown(): + logger.warning( + "Units of cube after executing derivation script of '%s' are " + "'%s', automatically setting them to '%s'. This might lead to " + "incorrect data", short_name, cube.units, units) + cube.units = units + elif cube.units.is_convertible(units): + cube.convert_units(units) + else: + raise ValueError( + f"Units '{cube.units}' after executing derivation script of " + f"'{short_name}' cannot be converted to target units '{units}'") + return cube diff --git a/tests/integration/preprocessor/_derive/test_interface.py b/tests/integration/preprocessor/_derive/test_interface.py index 7bc27858cc..41b423b506 100644 --- a/tests/integration/preprocessor/_derive/test_interface.py +++ b/tests/integration/preprocessor/_derive/test_interface.py @@ -1,12 +1,150 @@ +"""Integration tests for derive module.""" + +from unittest import mock + +import pytest +from cf_units import Unit from iris.cube import Cube, CubeList from esmvalcore.preprocessor import derive from esmvalcore.preprocessor._derive import get_required from esmvalcore.preprocessor._derive.ohc import DerivedVariable +SHORT_NAME = 'short_name' + + +class MockedCube(mock.MagicMock): + """Mock cube with correct convert_units fuction.""" + + def __init__(self): + """Initialize mock cube.""" + super().__init__(spec=Cube) + + +@pytest.fixture +def mock_cubes(): + """Fixture for mock version of :class:`iris.cube.CubeList`.""" + return mock.create_autospec(CubeList, spec_set=True, instance=True) + + +def mock_derived_var_dict(mock_dict, returned_units): + """Mock the :obj:`dict` containing all derived variables accordingly.""" + # cube = mock.create_autospec(Cube, instance=True) + cube = MockedCube() + cube.units = returned_units + calculate_function = mock.Mock(return_value=cube) + derived_var = mock.Mock(name='DerivedVariable') + derived_var.return_value.calculate = calculate_function + mock_dict.__getitem__.return_value = derived_var + + +def assert_derived_var_calc_called(mock_dict, *args): + """Assert that derivation script of variable has been called.""" + (mock_dict.__getitem__.return_value.return_value.calculate. + assert_called_once_with(*args)) + + +@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) +@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', + autospec=True) +@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) +def test_check_units_None(mock_logger, mock_all_derived_vars, _, mock_cubes): + """Test units after derivation if derivation scripts returns None.""" + mock_derived_var_dict(mock_all_derived_vars, None) + cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, + mock.sentinel.units, + standard_name=mock.sentinel.standard_name) + assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert cube.units == mock.sentinel.units + assert cube.var_name == SHORT_NAME + assert cube.long_name == mock.sentinel.long_name + assert cube.standard_name == mock.sentinel.standard_name + mock_logger.warning.assert_not_called() + cube.convert_units.assert_not_called() + + +@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) +@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', + autospec=True) +@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) +def test_check_units_equal(mock_logger, mock_all_derived_vars, _, mock_cubes): + """Test units after derivation if derivation scripts returns None.""" + mock_derived_var_dict(mock_all_derived_vars, Unit('kg m2 s-2')) + cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'J', + standard_name=mock.sentinel.standard_name) + assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert cube.units == Unit('J') + assert cube.var_name == SHORT_NAME + assert cube.long_name == mock.sentinel.long_name + assert cube.standard_name == mock.sentinel.standard_name + mock_logger.warning.assert_not_called() + cube.convert_units.assert_not_called() + + +@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) +@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', + autospec=True) +@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) +def test_check_units_nounit(mock_logger, mock_all_derived_vars, _, mock_cubes): + """Test units after derivation if derivation scripts returns None.""" + mock_derived_var_dict(mock_all_derived_vars, Unit('no unit')) + cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'J', + standard_name=mock.sentinel.standard_name) + assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert cube.units == Unit('J') + assert cube.var_name == SHORT_NAME + assert cube.long_name == mock.sentinel.long_name + assert cube.standard_name == mock.sentinel.standard_name + mock_logger.warning.assert_called_once_with( + "Units of cube after executing derivation script of '%s' are '%s', " + "automatically setting them to '%s'. This might lead to incorrect " + "data", SHORT_NAME, Unit('no_unit'), 'J') + cube.convert_units.assert_not_called() + + +@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) +@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', + autospec=True) +@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) +def test_check_units_unknown(mock_logger, mock_all_derived_vars, _, + mock_cubes): + """Test units after derivation if derivation scripts returns None.""" + mock_derived_var_dict(mock_all_derived_vars, Unit('unknown')) + cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'J', + standard_name=mock.sentinel.standard_name) + assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert cube.units == Unit('J') + assert cube.var_name == SHORT_NAME + assert cube.long_name == mock.sentinel.long_name + assert cube.standard_name == mock.sentinel.standard_name + mock_logger.warning.assert_called_once_with( + "Units of cube after executing derivation script of '%s' are '%s', " + "automatically setting them to '%s'. This might lead to incorrect " + "data", SHORT_NAME, Unit('unknown'), 'J') + cube.convert_units.assert_not_called() + + +@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) +@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', + autospec=True) +@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) +def test_check_units_convertible(mock_logger, mock_all_derived_vars, _, + mock_cubes): + """Test units after derivation if derivation scripts returns None.""" + mock_derived_var_dict(mock_all_derived_vars, Unit('kg s-1')) + cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'g yr-1', + standard_name=mock.sentinel.standard_name) + assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert cube.units == Unit('g yr-1') + assert cube.var_name == SHORT_NAME + assert cube.long_name == mock.sentinel.long_name + assert cube.standard_name == mock.sentinel.standard_name + mock_logger.warning.assert_not_called() + cube.convert_units.assert_not_called() -def test_get_required(): +def test_get_required(): + """Test getting required variables for derivation.""" variables = get_required('alb', 'CMIP5') reference = [ @@ -22,7 +160,7 @@ def test_get_required(): def test_get_required_with_fx(): - + """Test getting required variables for derivation with fx variables.""" variables = get_required('ohc', 'CMIP5') reference = [ @@ -34,7 +172,7 @@ def test_get_required_with_fx(): def test_derive_nonstandard_nofx(): - + """Test a specific derivation.""" short_name = 'alb' long_name = 'albedo at the surface' units = 1 @@ -60,7 +198,7 @@ def test_derive_nonstandard_nofx(): def test_derive_noop(): - + """Test derivation when it is not necessary.""" alb = Cube([1.]) alb.var_name = 'alb' alb.long_name = 'albedo at the surface' @@ -73,7 +211,7 @@ def test_derive_noop(): def test_derive_mixed_case_with_fx(tmp_path, monkeypatch): - + """Test derivation with fx file.""" short_name = 'ohc' long_name = 'Heat content in grid cell' units = 'J' From f9125e66af3be1c20858ad1edba5da6fd86e035e Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Wed, 19 Aug 2020 17:36:25 +0200 Subject: [PATCH 02/10] Fixed tests for variable derivation interface --- .../preprocessor/_derive/test_interface.py | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/tests/integration/preprocessor/_derive/test_interface.py b/tests/integration/preprocessor/_derive/test_interface.py index 41b423b506..27b1bb2c9a 100644 --- a/tests/integration/preprocessor/_derive/test_interface.py +++ b/tests/integration/preprocessor/_derive/test_interface.py @@ -13,14 +13,6 @@ SHORT_NAME = 'short_name' -class MockedCube(mock.MagicMock): - """Mock cube with correct convert_units fuction.""" - - def __init__(self): - """Initialize mock cube.""" - super().__init__(spec=Cube) - - @pytest.fixture def mock_cubes(): """Fixture for mock version of :class:`iris.cube.CubeList`.""" @@ -29,8 +21,7 @@ def mock_cubes(): def mock_derived_var_dict(mock_dict, returned_units): """Mock the :obj:`dict` containing all derived variables accordingly.""" - # cube = mock.create_autospec(Cube, instance=True) - cube = MockedCube() + cube = mock.create_autospec(Cube, instance=True) cube.units = returned_units calculate_function = mock.Mock(return_value=cube) derived_var = mock.Mock(name='DerivedVariable') @@ -48,7 +39,7 @@ def assert_derived_var_calc_called(mock_dict, *args): @mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', autospec=True) @mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) -def test_check_units_None(mock_logger, mock_all_derived_vars, _, mock_cubes): +def test_check_units_none(mock_logger, mock_all_derived_vars, _, mock_cubes): """Test units after derivation if derivation scripts returns None.""" mock_derived_var_dict(mock_all_derived_vars, None) cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, @@ -135,12 +126,28 @@ def test_check_units_convertible(mock_logger, mock_all_derived_vars, _, cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'g yr-1', standard_name=mock.sentinel.standard_name) assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) - assert cube.units == Unit('g yr-1') + cube.convert_units.assert_called_once_with('g yr-1') assert cube.var_name == SHORT_NAME assert cube.long_name == mock.sentinel.long_name assert cube.standard_name == mock.sentinel.standard_name mock_logger.warning.assert_not_called() - cube.convert_units.assert_not_called() + + +@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) +@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', + autospec=True) +@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) +def test_check_units_fail(mock_logger, mock_all_derived_vars, _, mock_cubes): + """Test units after derivation if derivation scripts returns None.""" + mock_derived_var_dict(mock_all_derived_vars, Unit('kg')) + with pytest.raises(ValueError) as err: + derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'm', + standard_name=mock.sentinel.standard_name) + assert str(err.value) == ( + "Units 'kg' after executing derivation script of 'short_name' cannot " + "be converted to target units 'm'" + ) + mock_logger.warning.assert_not_called() def test_get_required(): @@ -190,7 +197,6 @@ def test_derive_nonstandard_nofx(): alb = derive(cubes, short_name, long_name, units, standard_name) - print(alb) assert alb.var_name == short_name assert alb.long_name == long_name assert alb.units == units @@ -206,11 +212,10 @@ def test_derive_noop(): cube = derive([alb], alb.var_name, alb.long_name, alb.units) - print(cube) assert cube is alb -def test_derive_mixed_case_with_fx(tmp_path, monkeypatch): +def test_derive_mixed_case_with_fx(monkeypatch): """Test derivation with fx file.""" short_name = 'ohc' long_name = 'Heat content in grid cell' @@ -218,7 +223,7 @@ def test_derive_mixed_case_with_fx(tmp_path, monkeypatch): ohc_cube = Cube([]) - def mock_calculate(self, cubes): + def mock_calculate(_, cubes): assert len(cubes) == 1 assert cubes[0] == ohc_cube return Cube([]) From 5c70481535e9a92da20bdf842bb6eae78245d304 Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Fri, 21 Aug 2020 11:38:30 +0200 Subject: [PATCH 03/10] Adapted several derivation scripts --- esmvalcore/preprocessor/_derive/_shared.py | 5 +++-- esmvalcore/preprocessor/_derive/lvp.py | 11 +++++------ esmvalcore/preprocessor/_derive/sispeed.py | 9 ++++----- esmvalcore/preprocessor/_derive/toz.py | 13 +++++-------- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/esmvalcore/preprocessor/_derive/_shared.py b/esmvalcore/preprocessor/_derive/_shared.py index c3ca594ab7..260242ec9b 100644 --- a/esmvalcore/preprocessor/_derive/_shared.py +++ b/esmvalcore/preprocessor/_derive/_shared.py @@ -4,13 +4,14 @@ import iris +from esmvalcore.iris_helpers import var_name_constraint + logger = logging.getLogger(__name__) def cloud_area_fraction(cubes, tau_constraint, plev_constraint): """Calculate cloud area fraction for different parameters.""" - clisccp_cube = cubes.extract_strict( - iris.Constraint(name='isccp_cloud_area_fraction')) + clisccp_cube = cubes.extract_strict(var_name_constraint('clisccp')) new_cube = clisccp_cube new_cube = new_cube.extract(tau_constraint & plev_constraint) coord_names = [ diff --git a/esmvalcore/preprocessor/_derive/lvp.py b/esmvalcore/preprocessor/_derive/lvp.py index e3f6d7d2c3..84744766fc 100644 --- a/esmvalcore/preprocessor/_derive/lvp.py +++ b/esmvalcore/preprocessor/_derive/lvp.py @@ -4,7 +4,8 @@ - weig_ka """ -from iris import Constraint + +from esmvalcore.iris_helpers import var_name_constraint from ._baseclass import DerivedVariableBase @@ -31,11 +32,9 @@ def required(project): @staticmethod def calculate(cubes): """Compute Latent Heat Release from Precipitation.""" - hfls_cube = cubes.extract_strict( - Constraint(name='surface_upward_latent_heat_flux')) - pr_cube = cubes.extract_strict(Constraint(name='precipitation_flux')) - evspsbl_cube = cubes.extract_strict( - Constraint(name='water_evaporation_flux')) + hfls_cube = cubes.extract_strict(var_name_constraint('hfls')) + pr_cube = cubes.extract_strict(var_name_constraint('pr')) + evspsbl_cube = cubes.extract_strict(var_name_constraint('evspsbl')) lvp_cube = hfls_cube * (pr_cube / evspsbl_cube) diff --git a/esmvalcore/preprocessor/_derive/sispeed.py b/esmvalcore/preprocessor/_derive/sispeed.py index 5734b115b1..d53e4f111c 100644 --- a/esmvalcore/preprocessor/_derive/sispeed.py +++ b/esmvalcore/preprocessor/_derive/sispeed.py @@ -16,11 +16,10 @@ class DerivedVariable(DerivedVariableBase): @staticmethod def required(project): """Declare the variables needed for derivation.""" - required = [{ - 'short_name': 'usi', - }, { - 'short_name': 'vsi', - }] + if project == 'CMIP6': + required = [{'short_name': 'siu'}, {'short_name': 'siv'}] + else: + required = [{'short_name': 'usi'}, {'short_name': 'vsi'}] return required @staticmethod diff --git a/esmvalcore/preprocessor/_derive/toz.py b/esmvalcore/preprocessor/_derive/toz.py index 10a8adf388..e478250d5a 100644 --- a/esmvalcore/preprocessor/_derive/toz.py +++ b/esmvalcore/preprocessor/_derive/toz.py @@ -26,14 +26,10 @@ class DerivedVariable(DerivedVariableBase): @staticmethod def required(project): """Declare the variables needed for derivation.""" - required = [ - { - 'short_name': 'tro3' - }, - { - 'short_name': 'ps' - }, - ] + if project == 'CMIP6': + required = [{'short_name': 'o3'}, {'short_name': 'ps'}] + else: + required = [{'short_name': 'tro3'}, {'short_name': 'ps'}] return required @staticmethod @@ -64,6 +60,7 @@ def calculate(cubes): toz_cube = toz_cube / MW_O3 * AVOGADRO_CONST toz_cube.units = toz_cube.units / MW_O3_UNIT * AVOGADRO_CONST_UNIT toz_cube.convert_units(DOBSON_UNIT) + toz_cube.units = 'DU' return toz_cube From 1d04aaa0f055a123b2a7b9d2d2e79c8f40377825 Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Fri, 21 Aug 2020 11:41:25 +0200 Subject: [PATCH 04/10] Adapted sm derivation script for CMIP6 --- esmvalcore/preprocessor/_derive/sm.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esmvalcore/preprocessor/_derive/sm.py b/esmvalcore/preprocessor/_derive/sm.py index 45b3ddb074..91d505d0b9 100644 --- a/esmvalcore/preprocessor/_derive/sm.py +++ b/esmvalcore/preprocessor/_derive/sm.py @@ -2,7 +2,8 @@ import cf_units import numpy as np -from iris import Constraint + +from esmvalcore.iris_helpers import var_name_constraint from ._baseclass import DerivedVariableBase @@ -27,8 +28,7 @@ def calculate(cubes): 20 deg C). """ - mrsos_cube = cubes.extract_strict( - Constraint(name='moisture_content_of_soil_layer')) + mrsos_cube = cubes.extract_strict(var_name_constraint('mrsos')) depth = mrsos_cube.coord('depth').bounds.astype(np.float32) layer_thickness = depth[..., 1] - depth[..., 0] From 3ab4e39e39be4ba8a168a0a0bdce36ec1cf6bb38 Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Fri, 21 Aug 2020 15:43:25 +0200 Subject: [PATCH 05/10] Simplified tests using mocker fixture --- .../preprocessor/_derive/test_interface.py | 98 ++++++++----------- 1 file changed, 43 insertions(+), 55 deletions(-) diff --git a/tests/integration/preprocessor/_derive/test_interface.py b/tests/integration/preprocessor/_derive/test_interface.py index 27b1bb2c9a..14ea1d6c9a 100644 --- a/tests/integration/preprocessor/_derive/test_interface.py +++ b/tests/integration/preprocessor/_derive/test_interface.py @@ -6,7 +6,7 @@ from cf_units import Unit from iris.cube import Cube, CubeList -from esmvalcore.preprocessor import derive +from esmvalcore.preprocessor import _derive, derive from esmvalcore.preprocessor._derive import get_required from esmvalcore.preprocessor._derive.ohc import DerivedVariable @@ -19,127 +19,115 @@ def mock_cubes(): return mock.create_autospec(CubeList, spec_set=True, instance=True) -def mock_derived_var_dict(mock_dict, returned_units): +@pytest.fixture +def patched_derive(mocker): + """Fixture for mocked derivation scripts.""" + mocker.patch('iris.cube.CubeList', side_effect=lambda x: x) + mocker.patch.object(_derive, 'ALL_DERIVED_VARIABLES', autospec=True) + mocker.patch.object(_derive, 'logger', autospec=True) + + +def mock_all_derived_variables(returned_units): """Mock the :obj:`dict` containing all derived variables accordingly.""" cube = mock.create_autospec(Cube, instance=True) cube.units = returned_units calculate_function = mock.Mock(return_value=cube) derived_var = mock.Mock(name='DerivedVariable') derived_var.return_value.calculate = calculate_function - mock_dict.__getitem__.return_value = derived_var + _derive.ALL_DERIVED_VARIABLES.__getitem__.return_value = derived_var -def assert_derived_var_calc_called(mock_dict, *args): +def assert_derived_var_calc_called_once_with(*args): """Assert that derivation script of variable has been called.""" - (mock_dict.__getitem__.return_value.return_value.calculate. - assert_called_once_with(*args)) + (_derive.ALL_DERIVED_VARIABLES.__getitem__.return_value.return_value. + calculate.assert_called_once_with(*args)) -@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) -@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', - autospec=True) -@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) -def test_check_units_none(mock_logger, mock_all_derived_vars, _, mock_cubes): +@pytest.mark.usefixtures("patched_derive") +def test_check_units_none(mock_cubes): """Test units after derivation if derivation scripts returns None.""" - mock_derived_var_dict(mock_all_derived_vars, None) + mock_all_derived_variables(None) cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, mock.sentinel.units, standard_name=mock.sentinel.standard_name) - assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert_derived_var_calc_called_once_with(mock_cubes) assert cube.units == mock.sentinel.units assert cube.var_name == SHORT_NAME assert cube.long_name == mock.sentinel.long_name assert cube.standard_name == mock.sentinel.standard_name - mock_logger.warning.assert_not_called() + _derive.logger.warning.assert_not_called() cube.convert_units.assert_not_called() -@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) -@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', - autospec=True) -@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) -def test_check_units_equal(mock_logger, mock_all_derived_vars, _, mock_cubes): +@pytest.mark.usefixtures("patched_derive") +def test_check_units_equal(mock_cubes): """Test units after derivation if derivation scripts returns None.""" - mock_derived_var_dict(mock_all_derived_vars, Unit('kg m2 s-2')) + mock_all_derived_variables(Unit('kg m2 s-2')) cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'J', standard_name=mock.sentinel.standard_name) - assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert_derived_var_calc_called_once_with(mock_cubes) assert cube.units == Unit('J') assert cube.var_name == SHORT_NAME assert cube.long_name == mock.sentinel.long_name assert cube.standard_name == mock.sentinel.standard_name - mock_logger.warning.assert_not_called() + _derive.logger.warning.assert_not_called() cube.convert_units.assert_not_called() -@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) -@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', - autospec=True) -@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) -def test_check_units_nounit(mock_logger, mock_all_derived_vars, _, mock_cubes): +@pytest.mark.usefixtures("patched_derive") +def test_check_units_nounit(mock_cubes): """Test units after derivation if derivation scripts returns None.""" - mock_derived_var_dict(mock_all_derived_vars, Unit('no unit')) + mock_all_derived_variables(Unit('no unit')) cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'J', standard_name=mock.sentinel.standard_name) - assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert_derived_var_calc_called_once_with(mock_cubes) assert cube.units == Unit('J') assert cube.var_name == SHORT_NAME assert cube.long_name == mock.sentinel.long_name assert cube.standard_name == mock.sentinel.standard_name - mock_logger.warning.assert_called_once_with( + _derive.logger.warning.assert_called_once_with( "Units of cube after executing derivation script of '%s' are '%s', " "automatically setting them to '%s'. This might lead to incorrect " "data", SHORT_NAME, Unit('no_unit'), 'J') cube.convert_units.assert_not_called() -@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) -@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', - autospec=True) -@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) -def test_check_units_unknown(mock_logger, mock_all_derived_vars, _, - mock_cubes): +@pytest.mark.usefixtures("patched_derive") +def test_check_units_unknown(mock_cubes): """Test units after derivation if derivation scripts returns None.""" - mock_derived_var_dict(mock_all_derived_vars, Unit('unknown')) + mock_all_derived_variables(Unit('unknown')) cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'J', standard_name=mock.sentinel.standard_name) - assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert_derived_var_calc_called_once_with(mock_cubes) assert cube.units == Unit('J') assert cube.var_name == SHORT_NAME assert cube.long_name == mock.sentinel.long_name assert cube.standard_name == mock.sentinel.standard_name - mock_logger.warning.assert_called_once_with( + _derive.logger.warning.assert_called_once_with( "Units of cube after executing derivation script of '%s' are '%s', " "automatically setting them to '%s'. This might lead to incorrect " "data", SHORT_NAME, Unit('unknown'), 'J') cube.convert_units.assert_not_called() -@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) -@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', - autospec=True) -@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) -def test_check_units_convertible(mock_logger, mock_all_derived_vars, _, - mock_cubes): +@pytest.mark.usefixtures("patched_derive") +def test_check_units_convertible(mock_cubes): """Test units after derivation if derivation scripts returns None.""" - mock_derived_var_dict(mock_all_derived_vars, Unit('kg s-1')) + mock_all_derived_variables(Unit('kg s-1')) cube = derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'g yr-1', standard_name=mock.sentinel.standard_name) - assert_derived_var_calc_called(mock_all_derived_vars, mock_cubes) + assert_derived_var_calc_called_once_with(mock_cubes) cube.convert_units.assert_called_once_with('g yr-1') assert cube.var_name == SHORT_NAME assert cube.long_name == mock.sentinel.long_name assert cube.standard_name == mock.sentinel.standard_name - mock_logger.warning.assert_not_called() + _derive.logger.warning.assert_not_called() -@mock.patch('iris.cube.CubeList', side_effect=lambda x: x) -@mock.patch('esmvalcore.preprocessor._derive.ALL_DERIVED_VARIABLES', - autospec=True) -@mock.patch('esmvalcore.preprocessor._derive.logger', autospec=True) -def test_check_units_fail(mock_logger, mock_all_derived_vars, _, mock_cubes): +@pytest.mark.usefixtures("patched_derive") +def test_check_units_fail(mock_cubes): """Test units after derivation if derivation scripts returns None.""" - mock_derived_var_dict(mock_all_derived_vars, Unit('kg')) + mock_all_derived_variables(Unit('kg')) with pytest.raises(ValueError) as err: derive(mock_cubes, SHORT_NAME, mock.sentinel.long_name, 'm', standard_name=mock.sentinel.standard_name) @@ -147,7 +135,7 @@ def test_check_units_fail(mock_logger, mock_all_derived_vars, _, mock_cubes): "Units 'kg' after executing derivation script of 'short_name' cannot " "be converted to target units 'm'" ) - mock_logger.warning.assert_not_called() + _derive.logger.warning.assert_not_called() def test_get_required(): From 095481fb8fe655fdf30a8a8a91099cb72e01913c Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Fri, 2 Oct 2020 18:48:39 +0200 Subject: [PATCH 06/10] Fixed derivation of vegfrac and ohc --- esmvalcore/preprocessor/_derive/ohc.py | 1 + esmvalcore/preprocessor/_derive/vegfrac.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/esmvalcore/preprocessor/_derive/ohc.py b/esmvalcore/preprocessor/_derive/ohc.py index 5401a77017..77e67d3c8c 100644 --- a/esmvalcore/preprocessor/_derive/ohc.py +++ b/esmvalcore/preprocessor/_derive/ohc.py @@ -62,6 +62,7 @@ def calculate(cubes): # 2. multiply with each other and with cprho0 # some juggling with coordinates needed since Iris is very # restrictive in this regard + cube.convert_units('K') try: t_coord_dims = cube.coord_dims('time') except iris.exceptions.CoordinateNotFoundError: diff --git a/esmvalcore/preprocessor/_derive/vegfrac.py b/esmvalcore/preprocessor/_derive/vegfrac.py index a90af0cb22..992809013d 100644 --- a/esmvalcore/preprocessor/_derive/vegfrac.py +++ b/esmvalcore/preprocessor/_derive/vegfrac.py @@ -21,5 +21,5 @@ def calculate(cubes): baresoilfrac_cube = cubes.extract_strict( iris.Constraint(name='area_fraction')) - baresoilfrac_cube.data = 1. - baresoilfrac_cube.core_data() + baresoilfrac_cube.data = 100.0 - baresoilfrac_cube.core_data() return baresoilfrac_cube From a8bfd028a0a8b3f0f51af5ed8c38358d2a0423ce Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Thu, 8 Oct 2020 10:49:43 +0200 Subject: [PATCH 07/10] Fixed derivation of vegfrac --- esmvalcore/preprocessor/_derive/vegfrac.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/esmvalcore/preprocessor/_derive/vegfrac.py b/esmvalcore/preprocessor/_derive/vegfrac.py index 992809013d..1fee1e990f 100644 --- a/esmvalcore/preprocessor/_derive/vegfrac.py +++ b/esmvalcore/preprocessor/_derive/vegfrac.py @@ -1,6 +1,7 @@ """Derivation of variable `vegFrac`.""" -import iris +from esmvalcore.iris_helpers import var_name_constraint + from ._baseclass import DerivedVariableBase @@ -10,16 +11,20 @@ class DerivedVariable(DerivedVariableBase): @staticmethod def required(project): """Declare the variables needed for derivation.""" - required = [{ - 'short_name': 'baresoilFrac', - }] + required = [ + {'short_name': 'baresoilFrac'}, + {'short_name': 'residualFrac'}, + ] return required @staticmethod def calculate(cubes): """Compute vegetation fraction from bare soil fraction.""" - baresoilfrac_cube = cubes.extract_strict( - iris.Constraint(name='area_fraction')) + baresoilfrac_cube = cubes.extract_strict(var_name_constraint( + 'baresoilFrac')) + residualfrac = cubes.extract_strict(var_name_constraint( + 'residualFrac')) - baresoilfrac_cube.data = 100.0 - baresoilfrac_cube.core_data() + baresoilfrac_cube.data = (100.0 - baresoilfrac_cube.core_data() - + residualfrac.core_data()) return baresoilfrac_cube From fbd7defe3a3176561618fa1ecf0175ed081dd579 Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Thu, 8 Oct 2020 11:03:33 +0200 Subject: [PATCH 08/10] Fixed CMOR checker when coordinate standard_name is empty --- esmvalcore/cmor/check.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/esmvalcore/cmor/check.py b/esmvalcore/cmor/check.py index 3bf891803c..cbab6d420b 100644 --- a/esmvalcore/cmor/check.py +++ b/esmvalcore/cmor/check.py @@ -338,7 +338,10 @@ def _check_dim_names(self): else: try: cube_coord = self._cube.coord(var_name=coordinate.out_name) - if cube_coord.standard_name != coordinate.standard_name: + if (cube_coord.standard_name is None and + coordinate.standard_name == ''): + pass + elif cube_coord.standard_name != coordinate.standard_name: self.report_critical( self._attr_msg, coordinate.out_name, From 3233f574d13580f29f1224164ddfd06338f1cba1 Mon Sep 17 00:00:00 2001 From: Manuel Schlund Date: Fri, 9 Oct 2020 11:26:42 +0200 Subject: [PATCH 09/10] Fixed derivation of vegfrac --- esmvalcore/preprocessor/_derive/vegfrac.py | 28 +++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/esmvalcore/preprocessor/_derive/vegfrac.py b/esmvalcore/preprocessor/_derive/vegfrac.py index 1fee1e990f..5cb424c785 100644 --- a/esmvalcore/preprocessor/_derive/vegfrac.py +++ b/esmvalcore/preprocessor/_derive/vegfrac.py @@ -1,7 +1,11 @@ """Derivation of variable `vegFrac`.""" +import dask.array as da +import iris from esmvalcore.iris_helpers import var_name_constraint +from .._regrid import regrid + from ._baseclass import DerivedVariableBase @@ -14,6 +18,7 @@ def required(project): required = [ {'short_name': 'baresoilFrac'}, {'short_name': 'residualFrac'}, + {'short_name': 'sftlf', 'mip': 'fx'}, ] return required @@ -22,9 +27,26 @@ def calculate(cubes): """Compute vegetation fraction from bare soil fraction.""" baresoilfrac_cube = cubes.extract_strict(var_name_constraint( 'baresoilFrac')) - residualfrac = cubes.extract_strict(var_name_constraint( + residualfrac_cube = cubes.extract_strict(var_name_constraint( 'residualFrac')) + sftlf_cube = cubes.extract_strict(var_name_constraint('sftlf')) + + # Add time dimension to sftlf + target_shape_sftlf = (baresoilfrac_cube.shape[0], *sftlf_cube.shape) + sftlf_data = iris.util.broadcast_to_shape(sftlf_cube.data, + target_shape_sftlf, (1, 2)) + sftlf_cube = baresoilfrac_cube.copy(sftlf_data) + sftlf_cube.data = sftlf_cube.lazy_data() + + # Regrid sftlf if necessary and adapt mask + if sftlf_cube.shape != baresoilfrac_cube.shape: + sftlf_cube = regrid(sftlf_cube, baresoilfrac_cube, 'linear') + sftlf_cube.data = da.ma.masked_array( + sftlf_cube.core_data(), + mask=da.ma.getmaskarray(baresoilfrac_cube.core_data())) - baresoilfrac_cube.data = (100.0 - baresoilfrac_cube.core_data() - - residualfrac.core_data()) + # Calculate vegetation fraction + baresoilfrac_cube.data = (sftlf_cube.core_data() - + baresoilfrac_cube.core_data() - + residualfrac_cube.core_data()) return baresoilfrac_cube From dd571f5b61ed197a952024be069a7e351393eb26 Mon Sep 17 00:00:00 2001 From: Bouwe Andela Date: Fri, 9 Oct 2020 16:18:21 +0200 Subject: [PATCH 10/10] Update changelog --- doc/changelog.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 3590a8827e..3155039c66 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -12,8 +12,9 @@ Bug fixes - Fix diagnostic filter (`#713 `__) `Javier Vegas-Regidor `__ - Set unit=1 if anomalies are standardized (`#727 `__) `bascrezee `__ - Fix crash for FGOALS-g2 variables without longitude coordinate (`#729 `__) `Bouwe Andela `__ -- Improve variable alias managament (`#595 `__) `Javier Vegas-Regidor `__ +- Improve variable alias management (`#595 `__) `Javier Vegas-Regidor `__ - Fix area_statistics fx files loading (`#798 `__) `Javier Vegas-Regidor `__ +- Fix units after derivation (`#754 `__) `Manuel Schlund `__ Documentation ~~~~~~~~~~~~~