Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions improver/categorical/decision_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import copy
import operator
import warnings
from typing import Dict, List, Optional, Tuple, Union

import iris
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -276,14 +279,33 @@ 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 multiple thresholds are found, the closest threshold to the
# desired threshold, is chosen.
if num_thresholds > 1:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think an inline code comment here to describe how we handle finding multiple thresholds could be useful here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline comment added

raise ValueError(
diff = [
abs(point - threshold) for point in threshold_points
]
closest_point = threshold_points[diff.index(min(diff))]

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
)
used_cubes.append(closest_cube)

else:
used_cubes.extend(matched_threshold)

Expand Down
35 changes: 23 additions & 12 deletions improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py
Original file line number Diff line number Diff line change
Expand Up @@ -588,28 +588,39 @@ 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."""

# Modify the cube snowfall_rate to add an additional threshold
threshold_coord = find_threshold_coordinate(self.cubes[0])
additional_threshold = threshold_coord.points[0] * (
original_threshold = threshold_coord.points[0]
additional_threshold = original_threshold * (
1 + 0.5 * self.plugin.float_tolerance
)
threshold_coord.points = np.array(
[
threshold_coord.points[0],
original_threshold,
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 for snowfall_cubes
# We expect the original_threshold to be selected over the additional_threshold
expected_thresholds = {original_threshold, threshold_coord.points[2]}
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]
self.assertIn(cube_threshold_value, expected_thresholds)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be able to know which threshold we expect to be in the output cube, and to check that the actual returned threshold is the expected one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, there are multiple cubes in snowfall_cubes each its own threshold value, depending on its position on the decision tree. Here, we are asserting that we expect that this threshold value is either the original_threshold or threshold_coord.points[2], as the plugin will have selected the original_threshold over the additional_threshold.

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
Expand Down