From bc43de415499720980fd49c0e29075abb70ba215 Mon Sep 17 00:00:00 2001 From: SamGriffithsMO <122271903+SamGriffithsMO@users.noreply.github.com> Date: Thu, 1 Aug 2024 11:09:47 +0100 Subject: [PATCH] Migrate CLI functionality: simple_bias_correction (#2018) * refactored simple_bias_correction cli * docstring and contributing * changes for actions * sphinx building locally without error * whitespace change * bugfix: test upper_bound * changes based on review * review changes * review - doc-strings --- .mailmap | 1 + CONTRIBUTING.md | 1 + .../calibration/simple_bias_correction.py | 115 +++++++++++++----- improver/cli/apply_bias_correction.py | 30 +---- .../acceptance/test_apply_bias_correction.py | 67 ---------- .../test_ApplyBiasCorrection.py | 72 ++++++++++- 6 files changed, 155 insertions(+), 131 deletions(-) diff --git a/.mailmap b/.mailmap index 3959ea3131..4a4ca333f4 100644 --- a/.mailmap +++ b/.mailmap @@ -29,6 +29,7 @@ Meabh NicGuidhir <43375279+neilCrosswaite@users.noreply.github.com> Paul Abernethy Peter Jordan <52462411+mo-peterjordan@users.noreply.github.com> +Sam Griffiths <122271903+SamGriffithsMO@users.noreply.github.com> Shafiat Dewan <87321907+ShafiatDewan@users.noreply.github.com> <87321907+ShafiatDewan@users.noreply.github.com> Shubhendra Singh Chauhan Simon Jackson diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 22cf5c57cc..8dd2566698 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,6 +49,7 @@ below: - Zhiliang Fan (Bureau of Meteorology, Australia) - Ben Fitzpatrick (Met Office, UK) - Tom Gale (Bureau of Meteorology, Australia) + - Sam Griffiths (Met Office, UK) - Ben Hooper (Met Office, UK) - Aaron Hopkinson (Met Office, UK) - Kathryn Howard (Met Office, UK) diff --git a/improver/calibration/simple_bias_correction.py b/improver/calibration/simple_bias_correction.py index 23b71ebe42..a350ec3a77 100644 --- a/improver/calibration/simple_bias_correction.py +++ b/improver/calibration/simple_bias_correction.py @@ -4,7 +4,8 @@ # See LICENSE in the root of the repository for full licensing details. """Simple bias correction plugins.""" -from typing import Dict, Optional +import warnings +from typing import Dict, Optional, Union import iris import numpy.ma as ma @@ -12,6 +13,7 @@ from numpy import ndarray from improver import BasePlugin +from improver.calibration import add_warning_comment, split_forecasts_and_bias_files from improver.calibration.utilities import ( check_forecast_consistency, create_unified_frt_coord, @@ -23,6 +25,7 @@ create_new_diagnostic_cube, generate_mandatory_attributes, ) +from improver.utilities.common_input_handle import as_cubelist from improver.utilities.cube_manipulation import ( clip_cube_data, collapsed, @@ -248,11 +251,59 @@ class ApplyBiasCorrection(BasePlugin): the specified bias values. """ - def __init__(self): + def __init__( + self, + lower_bound: Optional[float] = None, + upper_bound: Optional[float] = None, + fill_masked_bias_values: bool = False, + ): """ Initialise class for applying simple bias correction. + + Args: + lower_bound: + A lower bound below which all values will be remapped to + after the bias correction step. + upper_bound: + An upper bound above which all values will be remapped to + after the bias correction step. + fill_masked_bias_values: + Flag to specify whether masked areas in the bias data + should be filled to an appropriate fill value. + """ + self._correction_method = apply_additive_correction + self._lower_bound = lower_bound + self._upper_bound = upper_bound + self._fill_masked_bias_values = fill_masked_bias_values + + def _split_forecasts_and_bias(self, cubes: CubeList): + """ + Wrapper for the split_forecasts_and_bias_files function. + + Args: + cubes: + Cubelist containing the input forecast and bias cubes. + + Return: + - Cube containing the forecast data to be bias-corrected. + - Cubelist containing the bias data to use in bias-correction. + Or None if no bias data is provided. """ - self.correction_method = apply_additive_correction + forecast_cube, bias_cubes = split_forecasts_and_bias_files(cubes) + + # Check whether bias data supplied, if not then return unadjusted input cube. + # This behaviour is to allow spin-up of the bias-correction terms. + if not bias_cubes: + msg = ( + "There are no forecast_error (bias) cubes provided for calibration. " + "The uncalibrated forecast will be returned." + ) + warnings.warn(msg) + forecast_cube = add_warning_comment(forecast_cube) + return forecast_cube, None + else: + bias_cubes = as_cubelist(bias_cubes) + return forecast_cube, bias_cubes def _get_mean_bias(self, bias_values: CubeList) -> Cube: """ @@ -302,6 +353,11 @@ def _check_forecast_bias_consistent( """Check that forecast and bias values are defined over the same valid-hour and forecast-period. + Checks that between the bias_data Cubes there is a common hour value for the + forecast_reference_time and single coordinate value for forecast_period. Then check + forecast Cube contains the same hour value for the forecast_reference_time and the + same forecast_period coordinate value. + Args: forecast: Cube containing forecast data to be bias-corrected. @@ -339,16 +395,8 @@ def _check_forecast_bias_consistent( "Forecast period differ between forecast and bias datasets." ) - def process( - self, - forecast: Cube, - bias: CubeList, - lower_bound: Optional[float] = None, - upper_bound: Optional[float] = None, - fill_masked_bias_values: Optional[bool] = False, - ) -> Cube: - """ - Apply bias correction using the specified bias values. + def process(self, *cubes: Union[Cube, CubeList],) -> Cube: + """Split then apply bias correction using the specified bias values. Where the bias data is defined point-by-point, the bias-correction will also be applied in this way enabling a form of statistical downscaling where coherent @@ -362,36 +410,37 @@ def process( filled using an appropriate fill value to leave the forecast data unchanged in the masked areas. + If no bias correction is provided, then the forecast is returned, unaltered. + Args: - forecast: - The cube to which bias correction is to be applied. - bias: - The cubelist containing the bias values for which to use in - the bias correction. - lower_bound: - A lower bound below which all values will be remapped to - after the bias correction step. - upper_bound: - An upper bound above which all values will be remapped to - after the bias correction step. - fill_masked_bias_values: - Flag to specify whether masked areas in the bias data - should be filled to an appropriate fill value. + cubes: + A list of cubes containing: + - A Cube containing the forecast to be calibrated. The input format is expected + to be realizations. + - A cube or cubelist containing forecast bias data over a specified + set of forecast reference times. If a list of cubes is passed in, each cube + should represent the forecast error for a single forecast reference time; the + mean value will then be evaluated over the forecast_reference_time coordinate. Returns: Bias corrected forecast cube. """ - self._check_forecast_bias_consistent(forecast, bias) - bias = self._get_mean_bias(bias) + cubes = as_cubelist(*cubes) + forecast, bias_cubes = self._split_forecasts_and_bias(cubes) + if bias_cubes is None: + return forecast + + self._check_forecast_bias_consistent(forecast, bias_cubes) + bias = self._get_mean_bias(bias_cubes) corrected_forecast = forecast.copy() - corrected_forecast.data = self.correction_method( - forecast, bias, fill_masked_bias_values + corrected_forecast.data = self._correction_method( + forecast, bias, self._fill_masked_bias_values ) - if (lower_bound is not None) or (upper_bound is not None): + if (self._lower_bound is not None) or (self._upper_bound is not None): corrected_forecast = clip_cube_data( - corrected_forecast, lower_bound, upper_bound + corrected_forecast, self._lower_bound, self._upper_bound ) return corrected_forecast diff --git a/improver/cli/apply_bias_correction.py b/improver/cli/apply_bias_correction.py index 5998c50014..2b9ed5ae38 100644 --- a/improver/cli/apply_bias_correction.py +++ b/improver/cli/apply_bias_correction.py @@ -12,7 +12,7 @@ @cli.clizefy @cli.with_output def process( - *input_cubes: cli.inputcube, + *cubes: cli.inputcube, lower_bound: float = None, upper_bound: float = None, fill_masked_bias_data: bool = False, @@ -32,7 +32,7 @@ def process( sensible post-bias correction. Args: - input_cubes (iris.cube.Cube or list of iris.cube.Cube): + cubes (iris.cube.Cube or list of iris.cube.Cube): A list of cubes containing: - A Cube containing the forecast to be calibrated. The input format is expected to be realizations. @@ -52,28 +52,8 @@ def process( iris.cube.Cube: Forecast cube with bias correction applied on a per member basis. """ - import warnings - - import iris - - from improver.calibration import add_warning_comment, split_forecasts_and_bias_files from improver.calibration.simple_bias_correction import ApplyBiasCorrection - forecast_cube, bias_cubes = split_forecasts_and_bias_files(input_cubes) - - # Check whether bias data supplied, if not then return unadjusted input cube. - # This behaviour is to allow spin-up of the bias-correction terms. - if not bias_cubes: - msg = ( - "There are no forecast_error (bias) cubes provided for calibration. " - "The uncalibrated forecast will be returned." - ) - warnings.warn(msg) - forecast_cube = add_warning_comment(forecast_cube) - return forecast_cube - else: - bias_cubes = iris.cube.CubeList(bias_cubes) - plugin = ApplyBiasCorrection() - return plugin.process( - forecast_cube, bias_cubes, lower_bound, upper_bound, fill_masked_bias_data - ) + return ApplyBiasCorrection(lower_bound, upper_bound, fill_masked_bias_data).process( + *cubes + ) diff --git a/improver_tests/acceptance/test_apply_bias_correction.py b/improver_tests/acceptance/test_apply_bias_correction.py index a86d83c8c1..d579559b90 100644 --- a/improver_tests/acceptance/test_apply_bias_correction.py +++ b/improver_tests/acceptance/test_apply_bias_correction.py @@ -134,70 +134,3 @@ def test_fill_masked_bias_data(tmp_path): ] run_cli(args) acc.compare(output_path, kgo_path) - - -def test_no_bias_file(tmp_path): - """ - Test case where no bias values are passed in. Expected behaviour is to - return the forecast value. - """ - kgo_dir = acc.kgo_root() / "apply-bias-correction" - fcst_path = kgo_dir / "20220814T0300Z-PT0003H00M-wind_speed_at_10m.nc" - kgo_path = kgo_dir / "fcst_with_comment" / "kgo.nc" - output_path = tmp_path / "output.nc" - args = [ - fcst_path, - "--output", - output_path, - ] - with pytest.warns(UserWarning, match=".*no forecast_error.*"): - run_cli(args) - acc.compare(output_path, fcst_path, exclude_attributes="comment") - acc.compare(output_path, kgo_path) - - -def test_missing_fcst_file(tmp_path): - """ - Test case where no forecast value has been passed in. This should raise - a ValueError. - """ - kgo_dir = acc.kgo_root() / "apply-bias-correction" - bias_file_path = ( - kgo_dir - / "single_bias_file" - / "bias_data" - / "20220813T0300Z-PT0003H00M-wind_speed_at_10m.nc" - ) - output_path = tmp_path / "output.nc" - args = [ - bias_file_path, - "--output", - output_path, - ] - with pytest.raises(ValueError, match="No forecast"): - run_cli(args) - - -def test_multiple_fcst_files(tmp_path): - """ - Test case where multiple forecast values are passed in. This should raise a - ValueError. - """ - kgo_dir = acc.kgo_root() / "apply-bias-correction" - fcst_path = kgo_dir / "20220814T0300Z-PT0003H00M-wind_speed_at_10m.nc" - bias_file_path = ( - kgo_dir - / "single_bias_file" - / "bias_data" - / "20220813T0300Z-PT0003H00M-wind_speed_at_10m.nc" - ) - output_path = tmp_path / "output.nc" - args = [ - fcst_path, - fcst_path, - bias_file_path, - "--output", - output_path, - ] - with pytest.raises(ValueError, match="Multiple forecast"): - run_cli(args) diff --git a/improver_tests/calibration/simple_bias_correction/test_ApplyBiasCorrection.py b/improver_tests/calibration/simple_bias_correction/test_ApplyBiasCorrection.py index f618224bbc..b807dcfdc2 100644 --- a/improver_tests/calibration/simple_bias_correction/test_ApplyBiasCorrection.py +++ b/improver_tests/calibration/simple_bias_correction/test_ApplyBiasCorrection.py @@ -4,6 +4,7 @@ # See LICENSE in the root of the repository for full licensing details. from datetime import datetime, timedelta +from unittest.mock import patch, sentinel import iris import numpy as np @@ -155,7 +156,7 @@ def test_apply_additive_correction( def test__init__(): """Test that the class functions are set to the expected values.""" plugin = ApplyBiasCorrection() - assert plugin.correction_method == apply_additive_correction + assert plugin._correction_method == apply_additive_correction @pytest.mark.parametrize("single_input_frt", (False, True)) @@ -257,7 +258,7 @@ def test_inconsistent_bias_forecast_inputs(forecast_cube, num_bias_inputs): @pytest.mark.parametrize("num_bias_inputs", (1, 30)) @pytest.mark.parametrize("single_input_frt", (False, True)) @pytest.mark.parametrize("lower_bound", (None, 1)) -@pytest.mark.parametrize("upper_bound", (None, 5)) +@pytest.mark.parametrize("upper_bound", (None, 4)) @pytest.mark.parametrize("masked_input_data", (True, False)) @pytest.mark.parametrize("masked_bias_data", (True, False)) @pytest.mark.parametrize("fill_masked_bias_data", (True, False)) @@ -282,12 +283,12 @@ def test_process( forecast_cube.data, dtype=forecast_cube.data.dtype ) forecast_cube.data.mask = MASK - result = ApplyBiasCorrection().process( - forecast_cube, - input_bias_cubelist, + result = ApplyBiasCorrection( lower_bound=lower_bound, + upper_bound=upper_bound, fill_masked_bias_values=fill_masked_bias_data, - ) + ).process(forecast_cube, input_bias_cubelist) + expected = TEST_FCST_DATA - MEAN_BIAS_DATA if fill_masked_bias_data and masked_bias_data: expected = np.where(MASK, TEST_FCST_DATA, expected) @@ -314,3 +315,62 @@ def test_process( # Check coords and attributes are consistent assert result.coords() == forecast_cube.coords() assert result.attributes == forecast_cube.attributes + + +class HaltExecution(Exception): + pass + + +@patch("improver.calibration.simple_bias_correction.as_cubelist") +def test_as_cubelist_called(mock_as_cubelist): + mock_as_cubelist.side_effect = HaltExecution + try: + ApplyBiasCorrection()(sentinel.cube1, sentinel.cube2, sentinel.cube3) + except HaltExecution: + pass + mock_as_cubelist.assert_called_once_with( + sentinel.cube1, sentinel.cube2, sentinel.cube3 + ) + + +def test_no_bias_file(forecast_cube): + """ + Test case where no bias values are passed in. Expected behaviour is to + return the forecast value. + """ + with pytest.warns(UserWarning, match=".*no forecast_error.*"): + result = ApplyBiasCorrection()(forecast_cube) + assert ( + "Warning: Calibration of this forecast has been attempted" + in result.attributes["comment"] + ) + + +def test_missing_fcst_file(): + """ + Test case where no forecast value has been passed in. This should raise + a ValueError. + """ + bias_cubes = generate_bias_cubelist( + 3, + last_valid_time=VALID_TIME + timedelta(hours=3), + single_frt_with_bounds=False, + ) + + with pytest.raises(ValueError, match="No forecast"): + ApplyBiasCorrection()(bias_cubes) + + +def test_multiple_fcst_files(forecast_cube): + """ + Test case where multiple forecast values are passed in. This should raise a + ValueError. + """ + bias_cubes = generate_bias_cubelist( + 3, + last_valid_time=VALID_TIME + timedelta(hours=3), + single_frt_with_bounds=False, + ) + + with pytest.raises(ValueError, match="Multiple forecast"): + ApplyBiasCorrection()(forecast_cube, forecast_cube, bias_cubes)