Fix snow falling level nan/masked issue (metoppv#1722)

* Fix edge case in falling level

* Style tweak...

* Address reviewer feedback

* Update conda-forge.yml

Propose pinning scipy to preserve unit test compatibility

* Update conda-forge.yml

* Update conda-forge.yml

* Update conda-forge.yml

* Update conda-forge.yml

* Update conda-forge.yml

* Update conda-forge.yml

* Implementation of proposed fix and test modifications.

Co-authored-by: benjamin.ayliffe <benjamin.ayliffe@metoffice.gov.uk>

envs/conda-forge.yml

@@ -18,7 +18,7 @@ dependencies:
   - iris>=3.0,<3.1
   - netCDF4
   - numpy<1.21
-  - scipy
+  - scipy<1.7
   - sigtools
   - sphinx
   # Additional libraries to run tests, not included in improver-feedstock

improver/psychrometric_calculations/psychrometric_calculations.py

@@ -966,19 +966,8 @@ def _calculate_phase_change_level(
         # lands points where the phase-change-level is below the orography.
         # These can be filled by optional horizontal interpolation.
         if self.horizontal_interpolation:
-            with np.errstate(invalid="ignore"):
-                max_nbhood_mask = phase_change_data <= max_nbhood_orog
-            updated_phase_cl = interpolate_missing_data(
-                phase_change_data, limit=orography, valid_points=max_nbhood_mask
-            )
-
-            with np.errstate(invalid="ignore"):
-                max_nbhood_mask = updated_phase_cl <= max_nbhood_orog
-            phase_change_data = interpolate_missing_data(
-                updated_phase_cl,
-                method="nearest",
-                limit=orography,
-                valid_points=max_nbhood_mask,
+            phase_change_data = self._horizontally_interpolate_phase(
+                phase_change_data, orography, max_nbhood_orog
             )
 
         # Mask any points that are still set to np.nan; this should be no
@@ -987,6 +976,47 @@ def _calculate_phase_change_level(
 
         return phase_change_data
 
+    def _horizontally_interpolate_phase(
+        self, phase_change_data: ndarray, orography: ndarray, max_nbhood_orog: ndarray
+    ) -> ndarray:
+        """
+        Fill in missing points via horizontal interpolation.
+
+        Args:
+            phase_change_data:
+                Level (height) at which the phase changes.
+            orography:
+                Orography heights
+            max_nbhood_orog:
+                Maximum orography height in neighbourhood (used to determine points that
+                can be used for interpolation)
+
+        Returns:
+            Level at which phase changes, with missing data filled in
+        """
+
+        with np.errstate(invalid="ignore"):
+            max_nbhood_mask = phase_change_data <= max_nbhood_orog
+        updated_phase_cl = interpolate_missing_data(
+            phase_change_data, limit=orography, valid_points=max_nbhood_mask
+        )
+
+        with np.errstate(invalid="ignore"):
+            max_nbhood_mask = updated_phase_cl <= max_nbhood_orog
+        phase_change_data = interpolate_missing_data(
+            updated_phase_cl,
+            method="nearest",
+            limit=orography,
+            valid_points=max_nbhood_mask,
+        )
+
+        if np.isnan(phase_change_data).any():
+            # This should be rare.
+            phase_change_data = interpolate_missing_data(
+                phase_change_data, method="nearest", limit=orography,
+            )
+        return phase_change_data
+
     def create_phase_change_level_cube(
         self, wbt: Cube, phase_change_level: ndarray
     ) -> Cube:

improver_tests/psychrometric_calculations/test_PhaseChangeLevel.py

@@ -133,7 +133,7 @@ class Test_fill_in_high_phase_change_falling_levels(IrisTest):
     """Test the fill_in_high_phase_change_falling_levels method."""
 
     def setUp(self):
-        """ Set up arrays for testing."""
+        """Set up arrays for testing."""
         self.phase_change_level_data = np.array(
             [[1.0, 1.0, 2.0], [1.0, np.nan, 2.0], [1.0, 2.0, 2.0]]
         )
@@ -159,7 +159,7 @@ def test_basic(self):
 
     def test_no_fill_if_conditions_not_met(self):
         """Test it doesn't fill in NaN if the heighest wet bulb integral value
-           is less than the threshold."""
+        is less than the threshold."""
         plugin = PhaseChangeLevel(phase_change="snow-sleet")
         expected = np.array([[1.0, 1.0, 2.0], [1.0, np.nan, 2.0], [1.0, 2.0, 2.0]])
         plugin.fill_in_high_phase_change_falling_levels(
@@ -290,7 +290,7 @@ class Test_fill_sea_points(IrisTest):
     """Test the fill_in_sea_points method."""
 
     def setUp(self):
-        """ Set up arrays for testing."""
+        """Set up arrays for testing."""
         self.phase_change_level = np.ones((3, 3)) * np.nan
         self.max_wb_integral = np.array(
             [[0.0, 0.0, 0.0], [0.0, 0.0, 0.0], [10.0, 10.0, 10.0]]
@@ -418,6 +418,178 @@ def test_null_lat_lon(self):
         self.assertArrayAlmostEqual(result.data, expected_data)
 
 
+class Test_horizontally_interpolate_phase(IrisTest):
+
+    """Test the PhaseChangeLevel horizontal interpolation."""
+
+    def setUp(self):
+        """Set up input data for our different cases.
+
+        The main aim in this set of unit tests is to test interpolation when
+        the phase change level data is largely above the maximum neighbourhood
+        orography, but also contains isolated NaNs. If the interpolation code
+        depends on interpolating low phase change level data (e.g. from
+        valleys) that is below the max orography, edge cases can arise where
+        the interpolation fails. This tests those edge cases.
+
+        The first case is one of the simplest possible - a 1d case with a NaN
+        in the middle, with two options to choose from either side.
+
+        The second case follows a real life example where the phase change
+        level was not determinable at a single point with steep gradients.
+
+        The third case is more deliberately contrived, with a large low
+        orography set of NaN points surrounded by a high orography set.
+
+        """
+
+        # A simple 1d case.
+        self.phase_change_data_1d = np.array(
+            [[1000.0, np.nan, 800.0]], dtype=np.float32
+        )
+        self.orography_1d = np.array([[850.0, 700.0, 500.0]], dtype=np.float32)
+        self.max_nbhood_orog_1d = np.array([[850.0, 850.0, 700.0]], dtype=np.float32)
+        self.expected_result_1d = np.array([[1000.0, 700.0, 800.0]], dtype=np.float32)
+
+        # A case that mimics a real side-of-mountain failure.
+        self.phase_change_data_2d = np.array(
+            [
+                [1000.0, 1000.0, 950.0, 800.0, 800.0],
+                [1000.0, 1000.0, 950.0, 900.0, 800.0],
+                [1000.0, 1000.0, 950.0, np.nan, 800.0],
+                [1000.0, 1000.0, 950.0, 900.0, 800.0],
+                [1000.0, 1000.0, 950.0, 800.0, 800.0],
+            ],
+            dtype=np.float32,
+        )
+        self.orography_2d = np.array(
+            [
+                [400.0, 500.0, 500.0, 500.0, 400.0],
+                [500.0, 700.0, 750.0, 700.0, 500.0],
+                [500.0, 700.0, 850.0, 700.0, 500.0],
+                [500.0, 700.0, 750.0, 700.0, 500.0],
+                [400.0, 500.0, 500.0, 500.0, 400.0],
+            ],
+            dtype=np.float32,
+        )
+        self.max_nbhood_orog_2d = np.array(
+            [
+                [700.0, 700.0, 700.0, 700.0, 700.0],
+                [700.0, 850.0, 850.0, 850.0, 700.0],
+                [700.0, 850.0, 850.0, 850.0, 700.0],
+                [700.0, 850.0, 850.0, 850.0, 700.0],
+                [700.0, 700.0, 700.0, 700.0, 700.0],
+            ],
+            dtype=np.float32,
+        )
+        self.expected_result_2d = np.array(
+            [
+                [1000.0, 1000.0, 950.0, 800.0, 800.0],
+                [1000.0, 1000.0, 950.0, 900.0, 800.0],
+                [1000.0, 1000.0, 950.0, 700.0, 800.0],
+                [1000.0, 1000.0, 950.0, 900.0, 800.0],
+                [1000.0, 1000.0, 950.0, 800.0, 800.0],
+            ],
+            dtype=np.float32,
+        )
+
+        # A 'NaN crater' with low orography circled by high orography.
+        self.phase_change_data_2d_crater = np.full((9, 9), 1000.0, dtype=np.float32)
+        self.phase_change_data_2d_crater[2:7, 2:7] = np.nan
+        self.orography_2d_crater = np.full((9, 9), 900.0, dtype=np.float32)
+        self.orography_2d_crater[2:7, 2:7] = 600.0
+        self.max_nbhood_orog_2d_crater = np.full((9, 9), 900.0, dtype=np.float32)
+        self.max_nbhood_orog_2d_crater[3:6, 3:6] = 600.0
+        self.expected_result_2d_crater = np.full((9, 9), 1000.0, dtype=np.float32)
+        self.expected_result_2d_crater[2:7, 2:7] = self.orography_2d_crater[2:7, 2:7]
+
+    def test_interpolate_edge_case_1d(self):
+        """Test that we fill in missing areas under a 1d peaked edge case."""
+        plugin = PhaseChangeLevel(phase_change="snow-sleet", grid_point_radius=1)
+        result = plugin._horizontally_interpolate_phase(
+            self.phase_change_data_1d, self.orography_1d, self.max_nbhood_orog_1d
+        )
+        self.assertArrayAlmostEqual(result, self.expected_result_1d)
+
+    def test_interpolate_edge_case_2d(self):
+        """Test that we fill in missing areas under a peaked edge case."""
+        plugin = PhaseChangeLevel(phase_change="snow-sleet", grid_point_radius=1)
+        result = plugin._horizontally_interpolate_phase(
+            self.phase_change_data_2d, self.orography_2d, self.max_nbhood_orog_2d
+        )
+        self.assertArrayAlmostEqual(result, self.expected_result_2d)
+
+    def test_interpolate_edge_case_2d_grid_point_radius_2(self):
+        """Test filling in missing areas under a radius 2 peaked edge case.
+
+        In this case, due to the higher max nbhood orog at the edges, we get
+        successful interpolation using the edge points as valid points, with
+        the orography at the nan point used as the limit.
+
+        """
+        plugin = PhaseChangeLevel(phase_change="snow-sleet", grid_point_radius=2)
+        max_nbhood_orog = np.full(
+            (5, 5), 850.0
+        )  # Determined from the grid point radius increase.
+        result = plugin._horizontally_interpolate_phase(
+            self.phase_change_data_2d, self.orography_2d, max_nbhood_orog
+        )
+        expected_result = self.expected_result_2d.copy()
+        expected_result[2][3] = self.orography_2d[2][
+            3
+        ]  # This uses the orography as the limit.
+        self.assertArrayAlmostEqual(result, expected_result)
+
+    def test_interpolate_edge_case_2d_nan_peak(self):
+        """Test that we fill in missing areas under a nan-peaked edge case."""
+        plugin = PhaseChangeLevel(phase_change="snow-sleet", grid_point_radius=1)
+        phase_change_data = self.phase_change_data_2d.copy()
+        phase_change_data[2][2] = np.nan  # Peak is also nan.
+        result = plugin._horizontally_interpolate_phase(
+            phase_change_data, self.orography_2d, self.max_nbhood_orog_2d
+        )
+        expected_result = self.expected_result_2d.copy()
+        expected_result[2][2] = self.orography_2d[2][2]
+        expected_result[2][3] = self.orography_2d[2][3]
+        self.assertArrayAlmostEqual(result, expected_result)
+
+    def test_interpolate_edge_case_2d_nan_peakonly(self):
+        """Test that we fill in missing areas under only-nan-peaked edge case."""
+        plugin = PhaseChangeLevel(phase_change="snow-sleet", grid_point_radius=1)
+        phase_change_data = self.phase_change_data_2d.copy()
+        phase_change_data[2][2] = np.nan
+        phase_change_data[2][3] = 950.0  # Just the peak is nan.
+        result = plugin._horizontally_interpolate_phase(
+            phase_change_data, self.orography_2d, self.max_nbhood_orog_2d
+        )
+        expected_result = self.expected_result_2d.copy()
+        expected_result[2][2] = self.orography_2d[2][2]
+        expected_result[2][3] = 950.0
+        self.assertArrayAlmostEqual(result, expected_result)
+
+    def test_interpolate_edge_case_2d_crater(self):
+        """Test that we fill in missing areas under a nan crater edge case."""
+        plugin = PhaseChangeLevel(phase_change="snow-sleet", grid_point_radius=1)
+        result = plugin._horizontally_interpolate_phase(
+            self.phase_change_data_2d_crater,
+            self.orography_2d_crater,
+            self.max_nbhood_orog_2d_crater,
+        )
+        self.assertArrayAlmostEqual(result, self.expected_result_2d_crater)
+
+    def test_interpolate_edge_case_2d_crater_grid_point_radius_2(self):
+        """Test filling in missing areas under a radius 2 nan crater edge case."""
+        plugin = PhaseChangeLevel(phase_change="snow-sleet", grid_point_radius=2)
+        max_nbhood_orog = np.full(
+            (9, 9), 900.0
+        )  # Determined from the grid point radius increase.
+        max_nbhood_orog[4, 4] = 600.0
+        result = plugin._horizontally_interpolate_phase(
+            self.phase_change_data_2d_crater, self.orography_2d_crater, max_nbhood_orog
+        )
+        self.assertArrayAlmostEqual(result, self.expected_result_2d_crater)
+
+
 class Test_process(IrisTest):
 
     """Test the PhaseChangeLevel processing works"""