IM-1787: Improve memory efficiency of threshold plugin

improver/cli/threshold.py

@@ -69,7 +69,7 @@ def process(
         threshold_config (dict):
             Threshold configuration containing threshold values and
             (optionally) fuzzy bounds. Best used in combination with
-            'threshold_units' It should contain a dictionary of strings that
+            'threshold_units'. It should contain a dictionary of strings that
             can be interpreted as floats with the structure:
             "THRESHOLD_VALUE": [LOWER_BOUND, UPPER_BOUND]
             e.g: {"280.0": [278.0, 282.0], "290.0": [288.0, 292.0]},
@@ -115,20 +115,9 @@ def process(
     Returns:
         iris.cube.Cube:
             Cube of probabilities relative to the given thresholds
-
-    Raises:
-        ValueError: If threshold_config and threshold_values are both set
-        ValueError: If threshold_config is used for fuzzy thresholding
     """
     from improver.threshold import Threshold
 
-    if threshold_config and threshold_values:
-        raise ValueError(
-            "--threshold-config and --threshold-values are mutually exclusive "
-            "- please set one or the other, not both"
-        )
-    if threshold_config and fuzzy_factor:
-        raise ValueError("--threshold-config cannot be used for fuzzy thresholding")
     if fill_masked is not None:
         fill_masked = float(fill_masked)
 

improver/precipitation_type/shower_condition_probability.py

@@ -172,11 +172,11 @@ def process(self, cubes: CubeList) -> Cube:
 
         # Threshold cubes
         cloud_thresholded = Threshold(
-            self.cloud_threshold, comparison_operator="<="
+            threshold_values=self.cloud_threshold, comparison_operator="<="
         ).process(cloud)
-        convection_thresholded = Threshold(self.convection_threshold).process(
-            convection
-        )
+        convection_thresholded = Threshold(
+            threshold_values=self.convection_threshold
+        ).process(convection)
 
         # Fill any missing data in the convective ratio field with zeroes.
         if np.ma.is_masked(convection_thresholded.data):

improver/regrid/landsea.py

@@ -288,9 +288,9 @@ def _get_matches(
         # Identify nearby points on regridded input_land that match the
         # selector_value
         if selector_val > 0.5:
-            thresholder = Threshold(0.5)
+            thresholder = Threshold(threshold_values=0.5)
         else:
-            thresholder = Threshold(0.5, comparison_operator="<=")
+            thresholder = Threshold(threshold_values=0.5, comparison_operator="<=")
         in_vicinity = self.vicinity(thresholder(self.input_land))
 
         # Identify those points sourced from the opposite mask that are

improver/threshold.py

@@ -79,7 +79,7 @@ def __init__(
         threshold_units: Optional[str] = None,
         comparison_operator: str = ">",
         collapse_coord: str = None,
-        vicinity: List[float] = None,
+        vicinity: Optional[Union[float, List[float]]] = None,
         fill_masked: Optional[float] = None,
     ) -> None:
         """
@@ -162,7 +162,14 @@ def __init__(
             ValueError: If both fuzzy_factor and fuzzy_bounds are set.
             ValueError: Can only collapse over a realization coordinate or a percentile
                         coordinate that has been rebadged as a realization coordinate.
+            ValueError: If threshold_config and threshold_values are both set
         """
+        if threshold_config and threshold_values:
+            raise ValueError(
+                "threshold_config and threshold_values are mutually exclusive "
+                "arguments - please provide one or the other, not both"
+            )
+
         thresholds, fuzzy_bounds = self._set_thresholds(
             threshold_values, threshold_config
         )
@@ -211,14 +218,14 @@ def __init__(
                 "Can only collapse over a realization coordinate or a percentile "
                 "coordinate that has been rebadged as a realization coordinate."
             )
-        else:
-            self.collapse_coord = collapse_coord
+        self.collapse_coord = collapse_coord
 
         self.vicinity = None
         if vicinity is not None:
             if isinstance(vicinity, numbers.Number):
                 vicinity = [vicinity]
-            self.vicinity = [float(x) for x in vicinity]
+            if vicinity:  # To handle an empty list being provided.
+                self.vicinity = [float(x) for x in vicinity]
 
         self.fill_masked = fill_masked
 
@@ -654,7 +661,7 @@ def process(self, input_cube: Cube, landmask: Cube = None) -> Cube:
             result = np.divide(dslice[valid], contribution_total[valid])
             thresholded_cube.data[i, valid] = result
 
-        if (contribution_total == 0).any():
+        if not valid.all():
             thresholded_cube.data = np.ma.masked_array(
                 thresholded_cube.data,
                 mask=np.broadcast_to(~valid, thresholded_cube.shape),

improver/utilities/spatial.py

@@ -474,10 +474,11 @@ def create_vicinity_coord(
     radius: Union[float, int], native_grid_point_radius: bool = False
 ) -> AuxCoord:
     """
-    Add a coordinate to the cube that records the vicinity radius that
-    has been applied to the data. This radius may be a distance in
-    physical units, or if native_grid_point_radius is True, it will be
-    a number of grid cells.
+    Create a coordinate that records the vicinity radius passed in.
+    This radius may be a distance in physical units, or if
+    native_grid_point_radius is True, it will be a number of grid cells.
+    If the latter an attribute comment is added to note that the radius
+    is in grid cells.
 
     Args:
         radius:

improver/utilities/textural.py

@@ -258,7 +258,9 @@ def process(self, input_cube: Cube) -> Cube:
             ratios.append(self._calculate_ratio(cslice, cube_name, self.nbhood_radius))
 
         ratios = ratios.merge_cube()
-        thresholded = Threshold(self.textural_threshold).process(ratios)
+        thresholded = Threshold(threshold_values=self.textural_threshold).process(
+            ratios
+        )
 
         # Squeeze scalar threshold coordinate.
         try:

improver_tests/threshold/conftest.py

@@ -52,7 +52,7 @@
 
 
 def diagnostic_cube(n_realizations: int = 1, data: Optional[np.ndarray] = None) -> Cube:
-    """Return a cube of preciptiation rate in mm/hr with n_realizations
+    """Return a cube of precipitation rate in mm/hr with n_realizations
     containing the data provided by the data argument.
 
     Args:
@@ -136,7 +136,7 @@ def custom_cube(n_realizations: int, data: np.ndarray) -> Cube:
 @pytest.fixture
 def expected_cube_name(
     comparison_operator: str, vicinity: Optional[Union[float, List[float]]]
-) -> Cube:
+) -> str:
     """Return a template for the name of a thresholded cube taking into
     account the comparison operator and the application of vicinity
     processing.
@@ -151,7 +151,7 @@ def expected_cube_name(
         The expected diagnostic name after thresholding.
     """
 
-    vicinity_name = "_in_vicinity" if vicinity is not None else ""
+    vicinity_name = "_in_vicinity" if vicinity else ""
 
     if "g" in comparison_operator.lower() or ">" in comparison_operator:
         return f"probability_of_{{cube_name}}{vicinity_name}_above_threshold"
@@ -169,20 +169,20 @@ def expected_result(
 ) -> np.ndarray:
     """Return the expected values following thresholding.
     The default cube has a 0.5 value at location (2, 2), with all other
-    values set to 0. The expected result, for a ">" or ">=" threshold
+    values set to 0. The expected result, for a "gt" or "ge" threshold
     comparator, is achieved by placing the expected value, or values for
     multi-realization data without coordinate collapsing, at the (..., 2, 2)
     location in an array full of zeros. This array matches the size of
     the input cube.
 
-    If the comparator is changed to be be "<" or "<=" then the array is
+    If the comparator is changed to be be "lt" or "le" then the array is
     filled with ones, and the expected value(s) is subtracted from 1 prior
-    to being places at the (..., 2, 2) location.
+    to being placed at the (..., 2, 2) location.
 
     This function does no more than this to avoid making the tests harder
     to follow. Cases for which the threshold value is equal to any of
-    the data values (without fuzziness) meaning that the ">" and ">=", or
-    "<" and "<=" comparators would give different results are handled
+    the data values (without fuzziness) meaning that the "gt" and "ge", or
+    "lt" and "le" comparators would give different results are handled
     separately.
 
     Args:
@@ -197,8 +197,8 @@ def expected_result(
             Whether the test includes the collapsing of the realization
             coordinate to calculate the final result.
         comparator:
-            The comapartor being applied in the thresholding, either ">",
-            ">=", "<", or "<=".
+            The comparator being applied in the thresholding, either "gt",
+            "ge", "lt", or "le".
         default_cube:
             The input cube to the test.
     Returns:

improver_tests/threshold/test_ThresholdPytest.py

@@ -102,9 +102,15 @@
             {"threshold_values": 0.6, "collapse_coord": "kittens"},
             "Can only collapse over a realization coordinate or a percentile",
         ),
+        # threshold values provided as argument and via config
+        (
+            {"threshold_values": 0.6, "threshold_config": {"0.6"}},
+            "threshold_config and threshold_values are mutually exclusive arguments",
+        ),
     ],
 )
 def test_init(kwargs, exception):
+    """Test the exceptions raised by the __init__ method."""
     with pytest.raises(ValueError, match=exception):
         Threshold(**kwargs)
 
@@ -144,11 +150,11 @@ def test__add_threshold_coord(default_cube, diagnostic, units):
     "n_realizations,data",
     [
         # A typical case with float inputs
-        (1, np.zeros((25), dtype=np.float32).reshape(5, 5)),
+        (1, np.zeros(25, dtype=np.float32).reshape(5, 5)),
         # A case with integer inputs, where the data is converted to
         # float32 type, allowing for non-integer thresholded values,
         # i.e. due to the application of fuzzy thresholds.
-        (1, np.zeros((25), dtype=np.int8).reshape(5, 5)),
+        (1, np.zeros(25, dtype=np.int8).reshape(5, 5)),
     ],
 )
 def test_attributes_and_types(custom_cube):
@@ -207,13 +213,25 @@ def test_threshold_metadata(
     assert result.name() == expected_cube_name.format(cube_name=ref_cube_name)
     assert result.coord(var_name="threshold") == threshold_coord
 
-    if vicinity is not None:
+    if vicinity is not None and vicinity:
         expected_vicinity = DimCoord(
             vicinity, long_name="radius_of_vicinity", units="m"
         )
         assert result.coord("radius_of_vicinity") == expected_vicinity
 
 
+def test_vicinity_as_empty_list(default_cube):
+    """Test that a vicinity set as an empty list does not result in
+    a vicinity diagnostic or any exceptions. Tested for a deterministic,
+    single realization, and multi-realization cube."""
+
+    expected_cube_name = "probability_of_precipitation_rate_above_threshold"
+    plugin = Threshold(threshold_values=0.5, vicinity=list())
+    result = plugin(default_cube)
+
+    assert result.name() == expected_cube_name
+
+
 @pytest.mark.parametrize("collapse", (False, True))
 @pytest.mark.parametrize("comparator", ("gt", "lt", "le", "ge"))
 @pytest.mark.parametrize(
@@ -310,7 +328,7 @@ def test_expected_values(default_cube, kwargs, collapse, comparator, expected_re
             {"threshold_config": {"0.": [-1, 1]}, "comparison_operator": "gt"},
             1,
             np.zeros((3, 3)),
-            np.ones((3, 3), dtype=np.float32) * 0.5,
+            np.full((3, 3), 0.5, dtype=np.float32),
         ),
         # negative diagnostic value above negative threshold with fuzziness.
         (
@@ -375,7 +393,7 @@ def test_expected_values(default_cube, kwargs, collapse, comparator, expected_re
             np.ones((3, 3), dtype=np.float32),
         ),
         # Vicinity processing, with one corner value exceeding the threshold
-        # resulting in neighbourhing cells probabilities of 1 within the limit
+        # resulting in neighbouring cells probabilities of 1 within the limit
         # of the defined vicinity radius (2x2km grid cells)
         (
             {"threshold_values": 0.5, "vicinity": 3000},
@@ -413,22 +431,15 @@ def test_expected_values(default_cube, kwargs, collapse, comparator, expected_re
                 [
                     np.stack(
                         [
-                            np.r_[[1] * 2, [0] * 3, [1] * 2, [0] * 18]
-                            .reshape((5, 5))
-                            .astype(np.float32),
-                            np.r_[[1] * 3, [0] * 2, [1] * 3, [0] * 2, [1] * 3, [0] * 12]
-                            .reshape((5, 5))
-                            .astype(np.float32),
-                        ]
-                    ),
-                    np.stack(
-                        [
-                            np.zeros((5, 5), dtype=np.float32),
-                            np.zeros((5, 5), dtype=np.float32),
+                            np.r_[[1] * 2, [0] * 3, [1] * 2, [0] * 18].reshape((5, 5)),
+                            np.r_[
+                                [1] * 3, [0] * 2, [1] * 3, [0] * 2, [1] * 3, [0] * 12
+                            ].reshape((5, 5)),
                         ]
                     ),
+                    np.zeros((2, 5, 5)),
                 ]
-            ),
+            ).astype(np.float32),
         ),
         # Multiple thresholds applied.
         (
@@ -448,6 +459,26 @@ def test_expected_values(default_cube, kwargs, collapse, comparator, expected_re
             np.ones((3, 3)),
             np.zeros((3, 3), dtype=np.float32),
         ),
+        # value of np.inf tested with an above and below comparator, as well as an
+        # equivalence test with a threshold set to np.inf.
+        (
+            {"threshold_values": 1.0e6, "comparison_operator": "gt"},
+            1,
+            np.array([[np.inf, 0], [0, 0]]),
+            np.array([[1.0, 0], [0, 0]], dtype=np.float32),
+        ),
+        (
+            {"threshold_values": 1.0e6, "comparison_operator": "lt"},
+            1,
+            np.array([[np.inf, 0], [0, 0]]),
+            np.array([[0, 1.0], [1.0, 1.0]], dtype=np.float32),
+        ),
+        (
+            {"threshold_values": np.inf, "comparison_operator": "ge"},
+            1,
+            np.array([[np.inf, 0], [0, 0]]),
+            np.array([[1.0, 0], [0, 0]], dtype=np.float32),
+        ),
     ],
 )
 def test_bespoke_expected_values(custom_cube, kwargs, expected_result):
@@ -532,6 +563,16 @@ def test_nan_handling(custom_cube):
         plugin(custom_cube)
 
 
+@pytest.mark.parametrize("n_realizations,data", [(1, np.array([[0, np.inf], [1, 1]]))])
+def test_inf_handling(custom_cube):
+    """Test that an exception is raised if the input data contains an
+    unmasked NaN."""
+
+    plugin = Threshold(threshold_values=1.0e6)
+    result = plugin(custom_cube)
+    print(result.data)
+
+
 @pytest.mark.parametrize(
     "n_realizations,data,mask", [(1, np.zeros((2, 2)), np.zeros((2, 2)))]
 )
@@ -597,3 +638,4 @@ def test_threshold_unit_conversion(default_cube):
         result.coord(var_name="threshold").points
         == np.array([3e-5, 9.0e-05, 1e-4], dtype="float32")
     ).all()
+    assert result.coord(var_name="threshold").units == "mm hr-1"