From f182dfd3fead27ad1ae5bb7d83c526650f9f4c26 Mon Sep 17 00:00:00 2001 From: Jasmine Beaver Date: Mon, 22 Jun 2026 15:09:46 +0100 Subject: [PATCH 1/3] Adding functionality to choose closest matching threshold. --- improver/categorical/decision_tree.py | 38 ++++++++++++++++--- .../decision_tree/test_ApplyDecisionTree.py | 31 +++++++++------ 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/improver/categorical/decision_tree.py b/improver/categorical/decision_tree.py index ed8299a71c..a9101e8529 100644 --- a/improver/categorical/decision_tree.py +++ b/improver/categorical/decision_tree.py @@ -6,6 +6,7 @@ import copy import operator +import warnings from typing import Dict, List, Optional, Tuple, Union import iris @@ -179,11 +180,13 @@ def prepare_input_cubes( indicated as allowed by the if_diagnostic_missing key. Raises: - ValueError: - Raises a ValueError if multiple matching thresholds are found. IOError: Raises an IOError if any of the required input data is missing. The error includes details of which fields are missing. + + Warnings: + - If multiple thresholds have been found and the closest matching threshold + will be used. """ cubes = as_cubelist(*cubes) @@ -276,14 +279,39 @@ def prepare_input_cubes( f"spp__relative_to_threshold: {condition}\n" ) else: - num_thresholds = len( + threshold_points = ( matched_threshold[0].coord(threshold_name).points ) + num_thresholds = len(threshold_points) + if num_thresholds > 1: - raise ValueError( + diff = [ + abs(point - threshold) for point in threshold_points + ] + diff_index = diff.index(min(diff)) + closest_point = threshold_points[diff_index] + + warnings.warn( f"Multiple ({num_thresholds}) matching thresholds found" - f" for name: {diagnostic}, threshold {threshold}" + f" for name: {diagnostic}, threshold {threshold}. " + f"Using closest match: {closest_point}" + ) + + closest_point_constraint = iris.Constraint( + coord_values={threshold_name: closest_point} ) + closest_cube = matched_threshold[0].extract( + closest_point_constraint + ) + + if closest_cube: + used_cubes.append(closest_cube) + else: + missing_data.append( + f"name: {diagnostic}, threshold: {threshold}, " + f"(closest match extraction failed)\n" + ) + else: used_cubes.extend(matched_threshold) diff --git a/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py b/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py index 715d72c4e6..05ca39c00a 100644 --- a/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py +++ b/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py @@ -588,28 +588,35 @@ def test_returns_used_cubes(self): for constraint in unexpected: self.assertEqual(len(result.extract(constraint)), 0) - def test_raises_error_matching_threshold(self): - """Test prepare_input_cubes method raises error for matching thresholds in a - diagnostic.""" + def test_selects_closest_threshold_when_multiple_matches(self): + """Test prepare_input_cubes method raises a warning for matching thresholds + in a diagnostic and selects the closest threshold.""" + threshold_coord = find_threshold_coordinate(self.cubes[0]) - additional_threshold = threshold_coord.points[0] * ( + original_threshold_coord = threshold_coord.points[0] + additional_threshold = original_threshold_coord * ( 1 + 0.5 * self.plugin.float_tolerance ) threshold_coord.points = np.array( [ - threshold_coord.points[0], + original_threshold_coord, additional_threshold, threshold_coord.points[2], ], dtype=np.float32, ) - msg = ( - r"Multiple \(2\) matching thresholds found for name: " - "probability_of_lwe_snowfall_rate_above_threshold" - ) - - with self.assertRaisesRegex(ValueError, msg): - self.plugin.prepare_input_cubes(self.cubes) + # Check warning is raised + msg = r"Multiple \(2\) matching thresholds found.*Using closest match" + with self.assertWarnsRegex(UserWarning, msg): + result, _ = self.plugin.prepare_input_cubes(self.cubes) + # Check thresholds extracted + extracted_thresholds = [ + cube.coord(threshold_coord.name()).points[0] + for cube in result + if threshold_coord.name() in [coord.name() for coord in cube.coords()] + ] + self.assertIn(original_threshold_coord, extracted_thresholds) + self.assertNotIn(additional_threshold, extracted_thresholds) def test_zero_threshold_uses_absolute_tolerance(self): """Test prepare_input_cubes method uses absolute tolerance when the threshold From 3562dbb4d66c96ce5eab19886975bb98b98115b1 Mon Sep 17 00:00:00 2001 From: Jasmine Beaver Date: Fri, 26 Jun 2026 14:58:44 +0100 Subject: [PATCH 2/3] Changes following review. --- improver/categorical/decision_tree.py | 14 +++-------- .../decision_tree/test_ApplyDecisionTree.py | 25 +++++++++++-------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/improver/categorical/decision_tree.py b/improver/categorical/decision_tree.py index a9101e8529..47e149124c 100644 --- a/improver/categorical/decision_tree.py +++ b/improver/categorical/decision_tree.py @@ -284,12 +284,13 @@ def prepare_input_cubes( ) num_thresholds = len(threshold_points) + # If multiple thresholds are found, the closest threshold to the + # desired threshold, is chosen. if num_thresholds > 1: diff = [ abs(point - threshold) for point in threshold_points ] - diff_index = diff.index(min(diff)) - closest_point = threshold_points[diff_index] + closest_point = threshold_points[diff.index(min(diff))] warnings.warn( f"Multiple ({num_thresholds}) matching thresholds found" @@ -303,14 +304,7 @@ def prepare_input_cubes( closest_cube = matched_threshold[0].extract( closest_point_constraint ) - - if closest_cube: - used_cubes.append(closest_cube) - else: - missing_data.append( - f"name: {diagnostic}, threshold: {threshold}, " - f"(closest match extraction failed)\n" - ) + used_cubes.append(closest_cube) else: used_cubes.extend(matched_threshold) diff --git a/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py b/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py index 05ca39c00a..95356ed34c 100644 --- a/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py +++ b/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py @@ -592,14 +592,15 @@ def test_selects_closest_threshold_when_multiple_matches(self): """Test prepare_input_cubes method raises a warning for matching thresholds in a diagnostic and selects the closest threshold.""" + # Modify the cube snowfall_rate to add an additional threshold threshold_coord = find_threshold_coordinate(self.cubes[0]) - original_threshold_coord = threshold_coord.points[0] - additional_threshold = original_threshold_coord * ( + original_threshold = threshold_coord.points[0] + additional_threshold = original_threshold * ( 1 + 0.5 * self.plugin.float_tolerance ) threshold_coord.points = np.array( [ - original_threshold_coord, + original_threshold, additional_threshold, threshold_coord.points[2], ], @@ -609,14 +610,16 @@ def test_selects_closest_threshold_when_multiple_matches(self): msg = r"Multiple \(2\) matching thresholds found.*Using closest match" with self.assertWarnsRegex(UserWarning, msg): result, _ = self.plugin.prepare_input_cubes(self.cubes) - # Check thresholds extracted - extracted_thresholds = [ - cube.coord(threshold_coord.name()).points[0] - for cube in result - if threshold_coord.name() in [coord.name() for coord in cube.coords()] - ] - self.assertIn(original_threshold_coord, extracted_thresholds) - self.assertNotIn(additional_threshold, extracted_thresholds) + # Check thresholds extracted for modified_cube + expected_thresholds = {original_threshold, threshold_coord.points[2]} + modified_cube = [cube for cube in result if "lwe_snowfall_rate" in cube.name()] + for cube in modified_cube: + self.assertGreater(len(modified_cube), 0) + cube_thresholds = cube.coord(find_threshold_coordinate(cube).name()).points + self.assertEqual(len(cube_thresholds), 1) + cube_threshold_value = cube_thresholds[0] + self.assertIn(cube_threshold_value, expected_thresholds) + self.assertNotEqual(cube_threshold_value, additional_threshold) def test_zero_threshold_uses_absolute_tolerance(self): """Test prepare_input_cubes method uses absolute tolerance when the threshold From 5b96933f24c3acfe2fdc5f88458aeafc579c99ed Mon Sep 17 00:00:00 2001 From: Jasmine Beaver Date: Mon, 6 Jul 2026 14:45:52 +0100 Subject: [PATCH 3/3] Small change to test following review. --- .../categorical/decision_tree/test_ApplyDecisionTree.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py b/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py index 95356ed34c..cd4c4c5883 100644 --- a/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py +++ b/improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py @@ -610,11 +610,12 @@ def test_selects_closest_threshold_when_multiple_matches(self): msg = r"Multiple \(2\) matching thresholds found.*Using closest match" with self.assertWarnsRegex(UserWarning, msg): result, _ = self.plugin.prepare_input_cubes(self.cubes) - # Check thresholds extracted for modified_cube + # Check thresholds extracted for snowfall_cubes + # We expect the original_threshold to be selected over the additional_threshold expected_thresholds = {original_threshold, threshold_coord.points[2]} - modified_cube = [cube for cube in result if "lwe_snowfall_rate" in cube.name()] - for cube in modified_cube: - self.assertGreater(len(modified_cube), 0) + snowfall_cubes = [cube for cube in result if "lwe_snowfall_rate" in cube.name()] + self.assertGreater(len(snowfall_cubes), 0) + for cube in snowfall_cubes: cube_thresholds = cube.coord(find_threshold_coordinate(cube).name()).points self.assertEqual(len(cube_thresholds), 1) cube_threshold_value = cube_thresholds[0]