From 7d936ec761cd44ebe6e488aca022d95d5bee6d15 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Sun, 3 May 2026 16:13:29 -0700 Subject: [PATCH] Validate mannings_n DataArray values in flood (#1437) travel_time and flood_depth_vegetation accept mannings_n as either a scalar or a DataArray. The scalar path enforced mannings_n > 0, but the DataArray path performed no validation. A roughness raster containing 0 silently produced inf velocity / 0 travel time; negatives produced negative travel time; NaN/Inf propagated. Add a _validate_mannings_n_dataarray helper that calls _validate_raster for structural checks and then verifies all values are finite and strictly positive. Wired into both travel_time (line 419) and flood_depth_vegetation (line 853). 6 new tests in TestMannigsNDataArrayValidation. --- xrspatial/flood.py | 21 +++++++++++ xrspatial/tests/test_flood.py | 68 +++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/xrspatial/flood.py b/xrspatial/flood.py index e0adb94a..43e1568c 100644 --- a/xrspatial/flood.py +++ b/xrspatial/flood.py @@ -38,6 +38,25 @@ # Minimum tan(slope) clamp: tan(0.001 deg), same as TWI _TAN_MIN = np.tan(np.radians(0.001)) + +def _validate_mannings_n_dataarray(mannings_n): + """Enforce finite, strictly positive values on a mannings_n DataArray. + + Mirrors the scalar-path check (`if mannings_n <= 0: raise`). Without + this guard, a roughness raster containing 0 silently produces inf + velocity / 0 travel time, and negatives produce negative travel time. + """ + _validate_raster(mannings_n, func_name='flood', + name='mannings_n', ndim=2) + arr = np.asarray(mannings_n.values) + if arr.size and ( + not np.isfinite(arr).all() or not (arr > 0).all() + ): + raise ValueError( + "mannings_n DataArray must contain finite, strictly positive " + "values (no zeros, negatives, NaN, or Inf)." + ) + # --------------------------------------------------------------------------- # NLCD-to-Manning's n lookup (Chow 1959; Arcement & Schneider 1989) # --------------------------------------------------------------------------- @@ -416,6 +435,7 @@ def travel_time( _validate_raster(slope_agg, func_name='travel_time', name='slope_agg') if isinstance(mannings_n, xr.DataArray): + _validate_mannings_n_dataarray(mannings_n) n_data = mannings_n.data elif isinstance(mannings_n, (int, float)): if mannings_n <= 0: @@ -850,6 +870,7 @@ def flood_depth_vegetation( raise ValueError("unit_discharge must be > 0") if isinstance(mannings_n, xr.DataArray): + _validate_mannings_n_dataarray(mannings_n) n_data = mannings_n.data elif isinstance(mannings_n, (int, float)): if mannings_n <= 0: diff --git a/xrspatial/tests/test_flood.py b/xrspatial/tests/test_flood.py index b30f4e91..fa697397 100644 --- a/xrspatial/tests/test_flood.py +++ b/xrspatial/tests/test_flood.py @@ -981,3 +981,71 @@ def test_numpy_equals_dask_cupy(self): general_output_checks(h_dc, result_dc, expected_results=result_np.data) + + +# ===================================================================== +# Issue #1437: mannings_n DataArray validation +# ===================================================================== + +class TestMannigsNDataArrayValidation: + """travel_time and flood_depth_vegetation enforce mannings_n>0 on DataArrays (#1437).""" + + @staticmethod + def _good_raster(value=1.0): + return xr.DataArray( + np.full((4, 4), value, dtype=np.float64), + dims=('y', 'x'), + coords={'y': np.arange(4), 'x': np.arange(4)}, + ) + + def test_travel_time_rejects_zero_mannings_n_dataarray(self): + from xrspatial.flood import travel_time + fl = self._good_raster(10.0) + slope = self._good_raster(5.0) + bad_n = self._good_raster(0.0) + with pytest.raises(ValueError, match="mannings_n"): + travel_time(fl, slope, bad_n) + + def test_travel_time_rejects_negative_mannings_n_dataarray(self): + from xrspatial.flood import travel_time + fl = self._good_raster(10.0) + slope = self._good_raster(5.0) + bad_n = self._good_raster(-0.05) + with pytest.raises(ValueError, match="mannings_n"): + travel_time(fl, slope, bad_n) + + def test_travel_time_rejects_nan_mannings_n_dataarray(self): + from xrspatial.flood import travel_time + fl = self._good_raster(10.0) + slope = self._good_raster(5.0) + bad_n = xr.DataArray( + np.array([[0.05, np.nan, 0.05, 0.05]] * 4, dtype=np.float64), + dims=('y', 'x'), + coords={'y': np.arange(4), 'x': np.arange(4)}, + ) + with pytest.raises(ValueError, match="mannings_n"): + travel_time(fl, slope, bad_n) + + def test_travel_time_accepts_valid_mannings_n_dataarray(self): + from xrspatial.flood import travel_time + fl = self._good_raster(10.0) + slope = self._good_raster(5.0) + good_n = self._good_raster(0.05) + result = travel_time(fl, slope, good_n) + assert result.shape == (4, 4) + + def test_flood_depth_vegetation_rejects_zero_mannings_n_dataarray(self): + from xrspatial.flood import flood_depth_vegetation + hand = self._good_raster(2.0) + slope = self._good_raster(0.5) + bad_n = self._good_raster(0.0) + with pytest.raises(ValueError, match="mannings_n"): + flood_depth_vegetation(hand, slope, bad_n, unit_discharge=1.0) + + def test_flood_depth_vegetation_rejects_negative_mannings_n_dataarray(self): + from xrspatial.flood import flood_depth_vegetation + hand = self._good_raster(2.0) + slope = self._good_raster(0.5) + bad_n = self._good_raster(-0.1) + with pytest.raises(ValueError, match="mannings_n"): + flood_depth_vegetation(hand, slope, bad_n, unit_discharge=1.0)