diff --git a/CHANGELOG.md b/CHANGELOG.md index c0f88de..9a0e0a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ uses Semantic Versioning for public releases. ## Unreleased ### Fixed +- Array summaries no longer report NaN for every statistic as soon as one + value is missing. `summarize_array` called plain `min`/`max`/`mean`, which + propagate NaN, so a field masked over half its faces returned `min`, `max` + and `mean` all NaN while three faces held finite values — and land masks are + ordinary in this data, so that was most fields. The statistics now skip + non-finite entries, report `None` rather than NaN when nothing is finite, and + add `n_finite`/`n_total` only when the two differ, so a complete field costs + no extra bytes. An infinity counts as missing rather than as an extreme, + since a maximum of `inf` is not a measurement. +- The same payloads were not valid JSON. `json.dumps` writes NaN as the bare + token `NaN`, which no JSON parser is required to accept, so a strict client + rejected the whole result rather than the one number: measured with + `json.loads(..., parse_constant=...)`, which raised on `{"min": NaN}`. - `temporal_mean` and `anomaly` now say how much time they averaged over. Measured on a 6-element, 12-step file: a variable missing at every step returned `outcome: complete`, `status: complete`, no warning codes and a diff --git a/src/uxarray_mcp/state.py b/src/uxarray_mcp/state.py index eb9bd2f..2a8b3b1 100644 --- a/src/uxarray_mcp/state.py +++ b/src/uxarray_mcp/state.py @@ -14,6 +14,7 @@ from pathlib import Path from typing import Any +import numpy as np import xarray as xr _WRITE_LOCK = threading.RLock() @@ -124,15 +125,49 @@ def summarize_grid(grid: Any) -> dict[str, Any]: def summarize_array(data: xr.DataArray) -> dict[str, Any]: - values = data.values + """Shape, dtype and statistics over the values that are actually there. + + The statistics skip non-finite entries, and say so when they had to. The + plain reductions this used to call propagate NaN, so a single missing + value made ``min``, ``max`` and ``mean`` all NaN -- measured on a temporal + mean whose field was masked over half its faces, where three faces held + finite means and the summary reported none of them. Land masks are + ordinary in this data, so that was most fields. + + ``NaN`` was also going out on the wire. ``json.dumps`` writes it as the + bare token ``NaN``, which is not JSON, and a client parsing strictly + rejects the payload rather than the number. Nothing finite reports + ``None`` instead. + + ``n_finite``/``n_total`` appear only when they differ. A count that is + always equal to the size costs payload on every call and tells the caller + nothing they could not read off ``shape``. + """ + values = np.asarray(data.values) summary: dict[str, Any] = { "dims": list(data.dims), "shape": list(data.shape), "dtype": str(data.dtype), "name": str(data.name) if data.name is not None else None, } - if values.size > 0: - with suppress(Exception): + if values.size == 0: + return summary + + with suppress(Exception): + if np.issubdtype(values.dtype, np.inexact): + finite = np.isfinite(values) + n_finite = int(finite.sum()) + if n_finite != values.size: + summary["n_finite"] = n_finite + summary["n_total"] = int(values.size) + usable = values[finite] + summary["min"] = float(usable.min()) if n_finite else None + summary["max"] = float(usable.max()) if n_finite else None + summary["mean"] = float(usable.mean()) if n_finite else None + else: + # Integers and booleans carry no missing value to skip, and + # datetimes raise on float() -- which the suppression handles the + # same way it did before. summary["min"] = float(values.min()) summary["max"] = float(values.max()) summary["mean"] = float(values.mean()) diff --git a/tests/test_array_summary.py b/tests/test_array_summary.py new file mode 100644 index 0000000..dbf9679 --- /dev/null +++ b/tests/test_array_summary.py @@ -0,0 +1,117 @@ +"""Array summaries describe the values that are there, in valid JSON. + +``summarize_array`` used plain ``min``/``max``/``mean``, which propagate NaN, +so one missing value emptied all three statistics. Measured on a temporal mean +of a field masked over half its faces: three faces held finite means and the +summary reported ``min``, ``max`` and ``mean`` all NaN, as ``outcome: +complete``. Land masks are ordinary in this data, so that was most fields. + +``json.dumps`` writes NaN as the bare token ``NaN``, which is not JSON, so the +same payloads were also unparseable by a strict client. +""" + +from __future__ import annotations + +import json + +import numpy as np +import pytest +import xarray as xr + +from uxarray_mcp.state import summarize_array + + +def _array(values, dims=("n_face",), **kwargs) -> xr.DataArray: + return xr.DataArray(np.asarray(values), dims=dims, **kwargs) + + +def _strict_json(payload) -> str: + """Serialise, refusing the non-JSON constants ``json`` emits by default.""" + + def reject(constant: str) -> None: + raise AssertionError(f"payload carries the non-JSON constant {constant}") + + text = json.dumps(payload) + json.loads(text, parse_constant=reject) + return text + + +def test_a_complete_field_summarises_as_before(): + summary = summarize_array(_array([1.0, 2.0, 3.0])) + assert summary["min"] == pytest.approx(1.0) + assert summary["max"] == pytest.approx(3.0) + assert summary["mean"] == pytest.approx(2.0) + assert summary["shape"] == [3] + # Nothing was skipped, so no count is added to every payload in the server. + assert "n_finite" not in summary + assert "n_total" not in summary + + +def test_a_masked_field_reports_the_values_it_has(): + summary = summarize_array(_array([1.0, np.nan, 3.0])) + assert summary["min"] == pytest.approx(1.0) + assert summary["max"] == pytest.approx(3.0) + assert summary["mean"] == pytest.approx(2.0) + assert summary["n_finite"] == 2 + assert summary["n_total"] == 3 + + +def test_an_infinity_counts_as_missing_rather_than_as_an_extreme(): + # A max of inf is not a measurement, and letting it through would make the + # range meaningless wherever a division by zero reached the field. + summary = summarize_array(_array([1.0, np.inf, 3.0])) + assert summary["max"] == pytest.approx(3.0) + assert summary["n_finite"] == 2 + + +def test_a_field_with_nothing_finite_reports_none_not_nan(): + summary = summarize_array(_array([np.nan, np.nan])) + assert summary["min"] is None + assert summary["max"] is None + assert summary["mean"] is None + assert summary["n_finite"] == 0 + assert summary["n_total"] == 2 + + +def test_a_masked_summary_is_valid_json(): + _strict_json(summarize_array(_array([1.0, np.nan]))) + + +def test_an_empty_summary_is_valid_json(): + _strict_json(summarize_array(_array([np.nan]))) + + +def test_an_empty_array_carries_no_statistics(): + summary = summarize_array(_array(np.array([], dtype=float))) + assert "min" not in summary + assert summary["shape"] == [0] + + +def test_an_integer_field_is_summarised_without_a_finite_mask(): + # Integers carry no missing value to skip, so the counts stay off. + summary = summarize_array(_array(np.array([1, 2, 3], dtype="int64"))) + assert summary["min"] == pytest.approx(1.0) + assert summary["mean"] == pytest.approx(2.0) + assert "n_finite" not in summary + + +def test_a_datetime_field_keeps_the_epoch_numbers_it_always_reported(): + # numpy casts datetime64 straight to nanoseconds since the epoch, so these + # statistics were already being returned and are left alone here. Pinning + # them keeps a later decision about what a time summary should say from + # happening by accident inside a NaN fix. + values = np.array(["2000-01-01", "2000-01-02"], dtype="datetime64[ns]") + summary = summarize_array(_array(values)) + assert summary["dtype"].startswith("datetime64") + assert summary["min"] == pytest.approx(9.466848e17) + assert "n_finite" not in summary + + +def test_a_multidimensional_field_counts_every_element(): + summary = summarize_array( + _array([[1.0, np.nan], [3.0, 4.0]], dims=("time", "n_face")) + ) + assert summary["shape"] == [2, 2] + assert summary["n_finite"] == 3 + assert summary["n_total"] == 4 + assert summary["mean"] == pytest.approx(8.0 / 3.0)