Skip to content

Commit 7142b03

Browse files
authored
Fix tasmax all-1e20 path in monthly formula workflow (#459)
* Fix tasmax all-1e20 path in monthly formula workflow * pre-commit * Increase patch coverage for tasmax 1e20 fixes * pre-commit * Cover non-numeric missing marker branch in calc utils
1 parent 40beac1 commit 7142b03

4 files changed

Lines changed: 203 additions & 3 deletions

File tree

src/access_moppy/atmosphere.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,23 @@ def select_and_process_variables(self):
163163
result = evaluate_expression(calc, context)
164164

165165
# Check whether the time interval/frequency has changed (e.g. daily → monthly)
166-
if "time" in result.dims and result.sizes["time"] != self.ds.sizes.get(
167-
"time", result.sizes["time"]
168-
):
166+
result_has_time = "time" in result.dims
167+
time_size_changed = result_has_time and result.sizes[
168+
"time"
169+
] != self.ds.sizes.get("time", result.sizes["time"])
170+
# Even when sizes match, assignment can align by coordinate labels.
171+
# If formula changes time labels (e.g. month-start -> month-midpoint),
172+
# direct assignment would reindex to NaN and later become 1e20.
173+
time_coord_changed = False
174+
if result_has_time and not time_size_changed and "time" in self.ds.coords:
175+
try:
176+
time_coord_changed = not np.array_equal(
177+
result["time"].values, self.ds["time"].values
178+
)
179+
except Exception:
180+
time_coord_changed = True
181+
182+
if time_size_changed or time_coord_changed:
169183
# If the temporal resolution changes, rebuild self.ds while preserving variables that are not time-dependent
170184
time_indep = {
171185
v: self.ds[v]

src/access_moppy/derivations/calc_utils.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,61 @@ def _monthly_midpoint_coord(time_da: xr.DataArray) -> xr.DataArray:
192192
return time_da.copy(data=midpoints)
193193

194194

195+
def _mask_missing_values_for_reduction(da: xr.DataArray) -> xr.DataArray:
196+
"""Mask configured missing-value sentinels so reductions ignore them.
197+
198+
Input files can carry numeric sentinels (for example ``1e20``) in data while
199+
storing marker values in attrs/encoding as ``_FillValue`` or ``missing_value``.
200+
If those sentinels are not masked, temporal maxima can collapse to the marker
201+
value. This helper applies a lazy mask using xarray operations, preserving
202+
Dask-backed arrays.
203+
"""
204+
205+
def _iter_markers(value):
206+
if value is None:
207+
return
208+
if np.isscalar(value):
209+
yield value
210+
return
211+
for v in np.ravel(value):
212+
yield v
213+
214+
markers = []
215+
has_nan_marker = False
216+
for container in (da.attrs, da.encoding):
217+
for key in ("missing_value", "_FillValue"):
218+
for raw in _iter_markers(container.get(key)):
219+
try:
220+
marker = float(raw)
221+
except (TypeError, ValueError):
222+
continue
223+
if np.isnan(marker):
224+
has_nan_marker = True
225+
else:
226+
markers.append(marker)
227+
228+
mask = None
229+
if np.issubdtype(da.dtype, np.floating) and has_nan_marker:
230+
mask = np.isnan(da)
231+
232+
for marker in set(markers):
233+
if np.isfinite(marker):
234+
# Match both exact values and float32-rounded encodings (e.g. 1e20).
235+
atol = max(1e-12, abs(float(np.spacing(np.float32(marker)))))
236+
condition = np.isclose(da, marker, rtol=0.0, atol=atol)
237+
else:
238+
condition = da == marker
239+
mask = condition if mask is None else (mask | condition)
240+
241+
if mask is None:
242+
return da
243+
244+
masked = da.where(~mask)
245+
masked.attrs = da.attrs.copy()
246+
masked.encoding = da.encoding.copy()
247+
return masked
248+
249+
195250
def calculate_monthly_minimum(
196251
da: xr.DataArray, time_dim: str = "time", preserve_attrs: bool = True
197252
) -> xr.DataArray:
@@ -262,6 +317,8 @@ def calculate_monthly_minimum(
262317
_name = da.name or "__tmp"
263318
da = xr.decode_cf(da.to_dataset(name=_name))[_name]
264319

320+
da = _mask_missing_values_for_reduction(da)
321+
265322
try:
266323
monthly_min = da.resample({time_dim: "ME"}).min(keep_attrs=preserve_attrs)
267324
# "ME" labels each bin at month-end; recentre to the cell midpoint
@@ -362,6 +419,8 @@ def calculate_monthly_maximum(
362419
_name = da.name or "__tmp"
363420
da = xr.decode_cf(da.to_dataset(name=_name))[_name]
364421

422+
da = _mask_missing_values_for_reduction(da)
423+
365424
try:
366425
monthly_max = da.resample({time_dim: "ME"}).max(keep_attrs=preserve_attrs)
367426
# "ME" labels each bin at month-end; recentre to the cell midpoint

tests/unit/test_atmosphere.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,6 +1102,81 @@ def test_formula_same_time_length_uses_setitem(self):
11021102

11031103
assert cmoriser.ds["tasmax"].sizes["time"] == 12
11041104

1105+
@pytest.mark.unit
1106+
def test_formula_same_time_length_but_shifted_labels_rebuilds(self):
1107+
"""Shifted time labels must not be aligned away to all-NaN values."""
1108+
monthly_time = pd.date_range("2020-01-01", periods=12, freq="MS")
1109+
monthly_ds = xr.Dataset(
1110+
{
1111+
"tasmax": xr.DataArray(
1112+
np.random.default_rng(5).normal(305, 5, 12),
1113+
dims=["time"],
1114+
coords={"time": monthly_time},
1115+
attrs={"units": "K"},
1116+
)
1117+
}
1118+
)
1119+
monthly_ds["time"].attrs = {"units": "days since 1850-01-01"}
1120+
1121+
shifted_time = pd.date_range("2020-01-16", periods=12, freq="MS")
1122+
shifted_result = xr.DataArray(
1123+
np.linspace(290.0, 301.0, 12),
1124+
dims=["time"],
1125+
coords={"time": shifted_time},
1126+
)
1127+
1128+
cmoriser = _make_cmoriser_for_formula(monthly_ds)
1129+
1130+
with patch(
1131+
"access_moppy.atmosphere.evaluate_expression",
1132+
return_value=shifted_result,
1133+
):
1134+
cmoriser.select_and_process_variables()
1135+
1136+
np.testing.assert_allclose(cmoriser.ds["tasmax"].values, shifted_result.values)
1137+
assert np.array_equal(cmoriser.ds["time"].values, shifted_result["time"].values)
1138+
1139+
@pytest.mark.unit
1140+
def test_formula_time_compare_exception_falls_back_to_rebuild(self):
1141+
"""If time-label comparison errors, fallback should still rebuild dataset."""
1142+
monthly_time = pd.date_range("2020-01-01", periods=12, freq="MS")
1143+
monthly_ds = xr.Dataset(
1144+
{
1145+
"tasmax": xr.DataArray(
1146+
np.random.default_rng(6).normal(305, 5, 12),
1147+
dims=["time"],
1148+
coords={"time": monthly_time},
1149+
attrs={"units": "K"},
1150+
)
1151+
}
1152+
)
1153+
monthly_ds["time"].attrs = {"units": "days since 1850-01-01"}
1154+
1155+
same_size_result = xr.DataArray(
1156+
np.linspace(280.0, 291.0, 12),
1157+
dims=["time"],
1158+
coords={"time": monthly_time},
1159+
)
1160+
1161+
cmoriser = _make_cmoriser_for_formula(monthly_ds)
1162+
1163+
with (
1164+
patch(
1165+
"access_moppy.atmosphere.evaluate_expression",
1166+
return_value=same_size_result,
1167+
),
1168+
patch(
1169+
"access_moppy.atmosphere.np.array_equal",
1170+
side_effect=RuntimeError("boom"),
1171+
),
1172+
):
1173+
cmoriser.select_and_process_variables()
1174+
1175+
np.testing.assert_allclose(
1176+
cmoriser.ds["tasmax"].values,
1177+
same_size_result.values,
1178+
)
1179+
11051180

11061181
class TestSoilDepthDimension:
11071182
"""

tests/unit/test_derivations_calc_utils.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import pytest
88
import xarray as xr
99

10+
from access_moppy.derivations import calc_utils as calc_utils_mod
1011
from access_moppy.derivations.calc_utils import (
1112
add_axis,
1213
calculate_monthly_maximum,
@@ -358,6 +359,17 @@ def test_values_are_monthly_maxima(self):
358359
# January maximum should be 99.0
359360
assert float(result.values[0]) == pytest.approx(99.0)
360361

362+
@pytest.mark.unit
363+
def test_ignores_fill_value_marker_in_maximum(self):
364+
times = xr.date_range("2000-01-01", periods=30, freq="D")
365+
data = np.linspace(10.0, 20.0, 30, dtype=np.float32)
366+
# Typical CF sentinel that otherwise dominates monthly maximum.
367+
data[5] = np.float32(1e20)
368+
da = xr.DataArray(data, dims=["time"], coords={"time": times})
369+
da.attrs["_FillValue"] = 1e20
370+
result = calculate_monthly_maximum(da)
371+
assert float(result.values[0]) == pytest.approx(20.0, rel=1e-6)
372+
361373
@pytest.mark.unit
362374
def test_raises_for_missing_time_dim(self):
363375
da = xr.DataArray(np.ones(4), dims=["lat"])
@@ -420,6 +432,46 @@ def test_resample_failure_raises_runtime_error(self):
420432
calculate_monthly_maximum(da)
421433

422434

435+
class TestMaskMissingValuesForReduction:
436+
@pytest.mark.unit
437+
def test_no_markers_returns_input_unchanged(self):
438+
da = xr.DataArray(np.array([1.0, 2.0, 3.0]), dims=["time"])
439+
out = calc_utils_mod._mask_missing_values_for_reduction(da)
440+
np.testing.assert_array_equal(out.values, da.values)
441+
442+
@pytest.mark.unit
443+
def test_masks_markers_from_encoding_and_iterable_fill_values(self):
444+
da = xr.DataArray(np.array([1.0, 99.0, np.inf]), dims=["time"])
445+
da.encoding["missing_value"] = 99.0
446+
# Exercise iterable marker path and non-finite marker comparison path.
447+
da.attrs["_FillValue"] = np.array([np.inf])
448+
449+
out = calc_utils_mod._mask_missing_values_for_reduction(da)
450+
451+
assert np.isnan(out.values[1])
452+
assert np.isnan(out.values[2])
453+
assert float(out.values[0]) == pytest.approx(1.0)
454+
455+
@pytest.mark.unit
456+
def test_nan_marker_masks_existing_nans(self):
457+
da = xr.DataArray(np.array([1.0, np.nan, 3.0]), dims=["time"])
458+
da.attrs["missing_value"] = np.nan
459+
460+
out = calc_utils_mod._mask_missing_values_for_reduction(da)
461+
462+
assert np.isnan(out.values[1])
463+
assert float(out.values[0]) == pytest.approx(1.0)
464+
465+
@pytest.mark.unit
466+
def test_ignores_non_numeric_marker_values(self):
467+
da = xr.DataArray(np.array([4.0, 5.0, 6.0]), dims=["time"])
468+
da.attrs["_FillValue"] = "not-a-number"
469+
470+
out = calc_utils_mod._mask_missing_values_for_reduction(da)
471+
472+
np.testing.assert_array_equal(out.values, da.values)
473+
474+
423475
# ---------------------------------------------------------------------------
424476
# Monthly time-coordinate midpoint (CF/CMIP6 "time squareness", TIME001)
425477
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)