apply_bias_correction fix: add handling for missing forecast cube (#…

…1995)

* Update the cli signature for bias-correction cli and add tests for missing files.

* Add function to separate forecast and forecast-error (bias) cubes

* Tidy up split_forecasts_and_bias_files function and update acceptance tests.

* Add unit tests for split_forecasts_and_bias_files function.

* Update the apply_bias_correction CLI to raise warning when no calibration is applied.

* Add unit test to check comment on output when no bias cubes supplied.

* Fix typos picked up in review.

improver/calibration/__init__.py

@@ -209,6 +209,45 @@ def split_forecasts_and_coeffs(
     )
 
 
+def split_forecasts_and_bias_files(cubes: CubeList) -> Tuple[Cube, Optional[CubeList]]:
+    """Split the input forecast from the forecast error files used for bias-correction.
+
+    Args:
+        cubes:
+            A list of input cubes which will be split into forecast and forecast errors.
+
+    Returns:
+        - A cube containing the current forecast.
+        - If found, a cube or cubelist containing the bias correction files.
+
+    Raises:
+        ValueError: If multiple forecast cubes provided, when only one is expected.
+        ValueError: If no forecast is found.
+    """
+    forecast_cube = None
+    bias_cubes = CubeList()
+
+    for cube in cubes:
+        if "forecast_error" in cube.name():
+            bias_cubes.append(cube)
+        else:
+            if forecast_cube is None:
+                forecast_cube = cube
+            else:
+                msg = (
+                    "Multiple forecast inputs have been provided. Only one is expected."
+                )
+                raise ValueError(msg)
+
+    if forecast_cube is None:
+        msg = "No forecast is present. A forecast cube is required."
+        raise ValueError(msg)
+
+    bias_cubes = bias_cubes if bias_cubes else None
+
+    return forecast_cube, bias_cubes
+
+
 def validity_time_check(forecast: Cube, validity_times: List[str]) -> bool:
     """Check the validity time of the forecast matches the accepted validity times
     within the validity times list.

improver/cli/apply_bias_correction.py

@@ -38,8 +38,7 @@
 @cli.clizefy
 @cli.with_output
 def process(
-    forecast_cube: cli.inputcube,
-    *bias_cubes: cli.inputcube,
+    *input_cubes: cli.inputcube,
     lower_bound: float = None,
     upper_bound: float = None,
     fill_masked_bias_data: bool = False,
@@ -55,14 +54,15 @@ def process(
     forecasts (from which the mean value is evaluated), or as a single bias value
     evaluated over a series of reference forecasts.
 
-    A lower bound can be set to ensure that corrected values are physically sensible
-    post-bias correction.
+    A lower bound or upper bound can be set to ensure that corrected values are physically
+    sensible post-bias correction.
 
     Args:
-        forecast_cube (iris.cube.Cube):
-            Cube containing the forecast to apply bias correction to.
-        bias_cubes (iris.cube.Cube or list of iris.cube.Cube):
-            A cube or list of cubes containing forecast bias data over the a specified
+        input_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.
+            - 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.
@@ -78,13 +78,24 @@ 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)

improver_tests/acceptance/SHA256SUMS

@@ -3,6 +3,7 @@ a6a84d0142796e4b9ca7bd3f0ad78586ea77684f5df02732fdda2ab54233cbb6  ./aggregate-re
 c0c38c5b1ba16fd5f7310b6d2ee0ab9d8b6bcdb3b23c8475193eefae1eb14a17  ./aggregate-reliability-tables/basic/reliability_table.nc
 9eeda326cdc66d93e591c87e9f7ceb17cea329397169fb0518f50da3bf4dc61f  ./aggregate-reliability-tables/basic/reliability_table_2.nc
 d82436afd61b2f9739e920c61293dc2bac32292f98255c86a632dd3dff504394  ./apply-bias-correction/20220814T0300Z-PT0003H00M-wind_speed_at_10m.nc
+5d259b136452c34a790d97359eebe0e250995463d040d24b77588e43236596cf  ./apply-bias-correction/fcst_with_comment/kgo.nc
 6b878745e994b0a1516a7b790983c129b87d8f53f6d0e1661e56b1f7ca9fc67a  ./apply-bias-correction/masked_bias_data/20220814T0300Z-PT0003H00M-wind_speed_at_10m.nc
 7e4b45553113d27002ab0a42a8b18bfa3b3a7d1d7afda3cd624fd419144ab29b  ./apply-bias-correction/masked_bias_data/bias_data/20220813T0300Z-PT0003H00M-wind_speed_at_10m.nc
 bd26f80cb7ae8e0592dd35ef5775e3a5b2f70365e18fc14eff0c8fdb2ed83e31  ./apply-bias-correction/masked_bias_data/fill_masked_values/kgo.nc

improver_tests/acceptance/test_apply_bias_correction.py

@@ -160,3 +160,70 @@ 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)

improver_tests/calibration/test_init.py

@@ -31,7 +31,7 @@
 """Unit tests for calibration.__init__"""
 
 import unittest
-from datetime import datetime
+from datetime import datetime, timedelta
 
 import iris
 import numpy as np
@@ -40,6 +40,7 @@
 
 from improver.calibration import (
     add_warning_comment,
+    split_forecasts_and_bias_files,
     split_forecasts_and_coeffs,
     split_forecasts_and_truth,
     validity_time_check,
@@ -542,6 +543,81 @@ def test_duplicate_forecasts(self):
             )
 
 
+@pytest.fixture
+def forecast_cube():
+    return set_up_variable_cube(
+        data=np.array(
+            [[1.0, 2.0, 2.0], [2.0, 1.0, 3.0], [1.0, 3.0, 3.0]], dtype=np.float32
+        ),
+        name="wind_speed",
+        units="m/s",
+    )
+
+
+@pytest.fixture
+def forecast_error_cubelist():
+    bias_cubes = CubeList()
+    for bias_index in range(-1, 2):
+        bias_cube = set_up_variable_cube(
+            data=np.array(
+                [[0.0, 0.0, 0.0], [-1.0, 1.0, 0.0], [-2.0, 0.0, 1.0]], dtype=np.float32
+            )
+            + (-1) * bias_index,
+            name="forecast_error_of_wind_speed",
+            units="m/s",
+            frt=(datetime(2017, 11, 10, 0, 0) - timedelta(days=(2 - bias_index))),
+            attributes={"title": "Forecast bias data"},
+        )
+        bias_cube.remove_coord("time")
+        bias_cubes.append(bias_cube)
+    return bias_cubes
+
+
+@pytest.mark.parametrize("multiple_bias_cubes", [(True, False)])
+def test_split_forecasts_and_bias_files(
+    forecast_cube, forecast_error_cubelist, multiple_bias_cubes
+):
+    """Test that split_forecasts_and_bias_files correctly separates out
+    the forecast cube from the forecast error cube(s)."""
+    if not multiple_bias_cubes:
+        forecast_error_cubelist = forecast_error_cubelist[0:]
+    merged_input_cubelist = forecast_error_cubelist.copy()
+    merged_input_cubelist.append(forecast_cube)
+
+    result_forecast_cube, result_bias_cubes = split_forecasts_and_bias_files(
+        merged_input_cubelist
+    )
+
+    assert result_forecast_cube == forecast_cube
+    assert result_bias_cubes == forecast_error_cubelist
+    if not multiple_bias_cubes:
+        assert len(result_bias_cubes) == 1
+
+
+@pytest.mark.parametrize("multiple_bias_cubes", [(True, False)])
+def test_split_forecasts_and_bias_files_missing_fcst(
+    forecast_error_cubelist, multiple_bias_cubes
+):
+    """Test that split_forecasts_and_bias_files raises a ValueError when
+    no forecast cube is provided."""
+    if not multiple_bias_cubes:
+        forecast_error_cubelist = forecast_error_cubelist[0:]
+    with pytest.raises(ValueError, match="No forecast is present"):
+        split_forecasts_and_bias_files(forecast_error_cubelist)
+
+
+def test_split_forecasts_and_bias_files_multiple_fcsts(
+    forecast_cube, forecast_error_cubelist
+):
+    """Test that split_forecasts_and_bias_files raises a ValueError when
+    multiple forecast cubes are provided."""
+    forecast_error_cubelist.append(forecast_cube)
+    forecast_error_cubelist.append(forecast_cube)
+
+    with pytest.raises(ValueError, match="Multiple forecast inputs"):
+        split_forecasts_and_bias_files(forecast_error_cubelist)
+
+
 @pytest.mark.parametrize(
     "time,validity_times,expected",
     [