diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b190aafbd..9004999a8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,11 +4,17 @@ ci: autoupdate_schedule: quarterly autoupdate_branch: pre-commit-autoupdate -# Ignore regression files as they should be kept as is +# Ignore committed regression *baseline data* — captured bundles kept byte-for-byte +# as produced, so they must not be reformatted or linted. +# This deliberately matches only data under a ``test-data/`` location; it must NOT +# match the regression *source* package (``climate_ref_core/regression/``) or its +# unit tests (``tests/unit/regression/``), which are linted like any other code. # See https://pre-commit.com/#regular-expressions exclude: | (?x)^( - .*/regression/.* + .*/test-data/.*/regression/.* + | tests/test-data/regression/.* + | tests/regression/.* )$ # See https://pre-commit.com/hooks.html for more hooks diff --git a/changelog/744.docs.md b/changelog/744.docs.md new file mode 100644 index 000000000..35eed948d --- /dev/null +++ b/changelog/744.docs.md @@ -0,0 +1 @@ +Corrected and expanded the regression-testing documentation, fixing inaccuracies in the diagnostic testing guide and filling gaps in the regression baselines reference. diff --git a/changelog/744.improvement.md b/changelog/744.improvement.md new file mode 100644 index 000000000..1a7c6bb44 --- /dev/null +++ b/changelog/744.improvement.md @@ -0,0 +1 @@ +Committed regression baseline bundles (`series.json` and `diagnostic.json`) now round floating-point values to seven significant figures when written, giving stable, reviewable bytes that no longer churn between local and CI runs while staying well within the regression comparison tolerance. diff --git a/changelog/744.trivial.md b/changelog/744.trivial.md new file mode 100644 index 000000000..3c4fb0ffd --- /dev/null +++ b/changelog/744.trivial.md @@ -0,0 +1 @@ +The pre-commit configuration now lints the `climate_ref_core.regression` source package and its tests; only the committed baseline data under `test-data/` remains excluded. diff --git a/docs/background/regression-baselines.md b/docs/background/regression-baselines.md index 7ab059b5c..1954f925f 100644 --- a/docs/background/regression-baselines.md +++ b/docs/background/regression-baselines.md @@ -27,6 +27,9 @@ A baseline is split into two layers with very different trust and portability pr The two layers are bound by a **`manifest.json`** alongside the bundle, which records: +- `schema` — integer schema version for the manifest format itself (currently `1`). + The loader rejects manifests whose `schema` does not match the current `SCHEMA_VERSION`, + so format migrations are detected immediately rather than silently misread. - `test_case_version` — a monotonic, author-bumped integer that *authorises* a new baseline. - `committed` — sha256 digests of the committed JSON artefacts, over the exact placeholder-substituted bytes on disk. - `native` — sha256 + size of each curated native file (empty `{}` until minted). @@ -62,6 +65,47 @@ flowchart LR | `mint` | write | Upload the curated native files to the object store and populate `manifest.native`. Generally run by CI. | | `sync` | public read | Fetch the native blobs referenced by the manifest into the local tree. | | `replay` | public read | Materialise the native baseline, re-run only `build_execution_result`, and tolerantly compare to the committed bundle. | +| `check-store` | write | Preflight the writable native store (credentials + bucket) without uploading anything. Run this before a slow `mint` to confirm credentials are correct. | + +## CMIP7 test data + +CMIP7 data is not yet available on ESGF, +so `CMIP7Request` (importable from `climate_ref_core.esgf`) bridges the gap: +it internally maps CMIP7 facets to their CMIP6 equivalents (e.g. `variant_label` -> `member_id`), +fetches the corresponding CMIP6 files, and converts them to CMIP7 format on the fly. + +Converted files are cached under the user cache directory at `climate_ref/cmip7-converted/` +(resolved by `platformdirs.user_cache_dir`), so repeated fetches for the same request are cheap. +The cache is not version-controlled; clear it manually if a conversion produces stale output. + +Use `CMIP7Request` in `test_data_spec` exactly like `CMIP6Request`: + +```python +from climate_ref_core.esgf import CMIP7Request + +CMIP7Request( + slug="cmip7-tas", + facets={ + "source_id": "ACCESS-ESM1-5", + "experiment_id": "historical", + "variable_id": "tas", + "variant_label": "r1i1p1f1", # CMIP7 name; maps to member_id internally + "table_id": "Amon", + }, +) +``` + +## Committed-bundle float precision + +Floats written into the committed bundle (`series.json`, `diagnostic.json`, `output.json`) +are rounded to **7 significant figures** at write time. +This keeps the committed bytes stable and human-reviewable across machines (local developer run vs. CI mint), +where tiny floating-point differences would otherwise produce noisy diffs on every baseline update. + +Seven significant figures is deliberately one digit finer than the `rtol=1e-6` +tolerance used by the coupling gate's tolerant comparison, +so rounding at write time can never flip a gate verdict: +a value that rounds identically on all machines will always compare within tolerance. ## Tolerant comparison diff --git a/docs/how-to-guides/testing-diagnostics.md b/docs/how-to-guides/testing-diagnostics.md index 5b758f9b9..9cafc6f3d 100644 --- a/docs/how-to-guides/testing-diagnostics.md +++ b/docs/how-to-guides/testing-diagnostics.md @@ -102,13 +102,12 @@ class MyDiagnostic(Diagnostic): ### TestCase Attributes -| Attribute | Type | Description | -| --------------- | ---------------------------- | -------------------------------------------------------------- | -| `name` | `str` | Unique identifier (e.g., `"default"`, `"edge-case"`) | -| `description` | `str` | Human-readable description of the test scenario | -| `requests` | `tuple[ESGFRequest, ...]` | ESGF requests to fetch the required datasets for the test case | -| `datasets` | `ExecutionDatasetCollection` | Explicit datasets (highest priority) | -| `datasets_file` | `str` | Path to YAML file with datasets (relative to package) | +| Attribute | Type | Description | +| --------------- | ------------------------- | -------------------------------------------------------------- | +| `name` | `str` | Unique identifier (e.g., `"default"`, `"edge-case"`) | +| `description` | `str` | Human-readable description of the test scenario | +| `requests` | `tuple[ESGFRequest, ...]` | ESGF requests to fetch the required datasets for the test case | +| `datasets_file` | `str \| None` | Path to a pre-built catalog YAML file (relative to package) | ### ESGF Requests @@ -169,13 +168,18 @@ Obs4MIPsRequest( | `institution_id` | Institution | `"CSIRO"` | | `activity_drs` | Activity | `"CMIP"`, `"ScenarioMIP"` | -### Dataset Resolution Priority +### Dataset Resolution -When running a test case, datasets are resolved in this order: +A `TestCase` resolves its datasets via one of two mechanisms: -1. **Explicit `datasets`**: If provided, used directly -2. **`datasets_file`**: Load from YAML file -3. **Solve from catalog**: Use `requests` to filter available data from the requests and solved +- **`datasets_file`**: If set, datasets are loaded directly from the specified YAML file. + Use this when you have pre-built catalog data at a known location + or when you need precise, machine-independent control over which files are used. +- **Solve from catalog**: If `datasets_file` is not set, the test runner uses `requests` + to filter and solve datasets from the local catalog (populated by `ref test-cases fetch`). + +Only datasets resolved by the active mechanism are visible during the test run, +ensuring reproducible execution regardless of what other data is present locally. ### Using a Datasets File @@ -405,12 +409,21 @@ Regenerate the committed bundle when you add a diagnostic or intend to change it ref test-cases run --provider my-provider --diagnostic my-diagnostic --force-regen ``` -We keep committed files small — a pre-commit hook flags anything over a few MB. +We keep committed files small. +After `ref test-cases run`, `_print_regression_summary` reports any file in the +`regression/` directory that exceeds the `--size-threshold` (default 1.0 MB). Large outputs belong in the native bundle, published with `mint` (see below). -### Comparing against the baseline +/// note +The pre-commit `check-added-large-files` hook does **not** flag regression baselines — +`.*/regression/.*` is explicitly excluded in `.pre-commit-config.yaml`. +Size enforcement for regression files comes solely from `ref test-cases run`. +/// -The `execution_regression` fixture compares results automatically: +### Updating the baseline in pytest + +The `execution_regression` fixture is a **factory** — call it with your diagnostic to +get an `ExecutionRegression` instance, then call `.check(key, output_directory)` on that: ```python def test_regression(run_test_case, execution_regression): @@ -419,10 +432,16 @@ def test_regression(run_test_case, execution_regression): diagnostic = MyDiagnostic() result = run_test_case.run(diagnostic, "default") - # Compare metric bundle against baseline - execution_regression.check(result, "my-provider/my-diagnostic/default") + regression = execution_regression(diagnostic) + regression.check("default", result.output_directory) ``` +`ExecutionRegression.check` only **regenerates** the committed bundle when pytest is +invoked with `--force-regen`. +It does not compare the result against the stored baseline itself — +that comparison is handled by the CLI (`ref test-cases run`) and the CI gate. +Use `--force-regen` only when you intend to record a new baseline. + ### The pull request workflow When you open a pull request, CI decides *how* to verify each test case from what your diff --git a/packages/climate-ref-core/src/climate_ref_core/regression/_quantise.py b/packages/climate-ref-core/src/climate_ref_core/regression/_quantise.py new file mode 100644 index 000000000..272d66c0c --- /dev/null +++ b/packages/climate-ref-core/src/climate_ref_core/regression/_quantise.py @@ -0,0 +1,66 @@ +""" +Float quantisation for committed regression bundles. + +Committed regression JSON (``series.json`` / ``diagnostic.json`` / ``output.json``) +records full-precision floats whose least-significant digits are platform-dependent +(CPU, BLAS, library versions). +Those last digits churn byte-for-byte between CI and local runs even when the result is numerically identical, +producing noisy, unreviewable diffs in the committed bundle. + +Rounding every float to a fixed number of significant figures at write time gives +stable, reviewable committed bytes. +We round to seven significant figures: one digit finer than the regression compare +tolerance (``rtol=1e-6`` in :mod:`climate_ref_core.regression.compare`), +so the rounding error stays an order of magnitude under tolerance and can never flip a boundary gate verdict. + +This affects only the committed bundle. +The native blobs (``.nc`` / ``.png``) and their content-addressed digests are never touched. +""" + +from __future__ import annotations + +from typing import Any + +DEFAULT_SIG_FIGS: int = 7 +"""Default significant figures for committed-bundle floats. + +Deliberately one digit finer than the ``rtol=1e-6`` regression compare tolerance, +so rounding never flips a boundary gate verdict. +""" + + +def round_floats(obj: Any, sig_figs: int = DEFAULT_SIG_FIGS) -> Any: + """ + Recursively round every ``float`` in a JSON-like structure to ``sig_figs`` figures. + + Walks dicts, lists and tuples, rounding each ``float`` via the ``g`` format + (``float(f"{x:.{sig_figs}g}")``) and leaving ``int``, ``bool``, ``str`` and ``None`` untouched. + ``bool`` is a subclass of ``int`` (and not of ``float``), so booleans are never rounded. + The operation is idempotent: rounding an already-rounded value is a no-op. + + Tuples are returned as lists, matching JSON serialisation semantics + (JSON has no tuple type; the standard library serialises tuples as arrays). + + Parameters + ---------- + obj + A JSON-like object: a scalar, or an arbitrarily nested dict / list / tuple. + sig_figs + The number of significant figures to round each float to. + + Returns + ------- + : + A copy of ``obj`` with every float rounded to ``sig_figs`` significant figures. + """ + # ``bool`` is a subclass of ``int``; + # check it explicitly so booleans are preserved rather than being coerced through the float branch. + if isinstance(obj, bool): + return obj + if isinstance(obj, float): + return float(f"{obj:.{sig_figs}g}") + if isinstance(obj, dict): + return {key: round_floats(value, sig_figs) for key, value in obj.items()} + if isinstance(obj, (list, tuple)): + return [round_floats(item, sig_figs) for item in obj] + return obj diff --git a/packages/climate-ref-core/src/climate_ref_core/regression/capture.py b/packages/climate-ref-core/src/climate_ref_core/regression/capture.py index 2f81ddb72..4c2bb781c 100644 --- a/packages/climate-ref-core/src/climate_ref_core/regression/capture.py +++ b/packages/climate-ref-core/src/climate_ref_core/regression/capture.py @@ -20,6 +20,7 @@ from __future__ import annotations +import json import shutil from pathlib import Path from typing import TYPE_CHECKING @@ -27,6 +28,7 @@ from climate_ref_core.output_files import copy_execution_outputs, to_placeholders from climate_ref_core.paths import safe_path +from ._quantise import round_floats from .manifest import ( COMMITTED_BUNDLE_FILES, NativeEntry, @@ -39,6 +41,48 @@ from climate_ref_core.regression.store import NativeStore +# Per-file serialisation parameters for the committed bundle, +# mirroring exactly how each artefact is written natively. +_COMMITTED_FLOAT_JSON_KWARGS: dict[str, dict[str, object]] = { + "series.json": {"indent": 2, "allow_nan": False, "sort_keys": True}, + "diagnostic.json": {"indent": 2, "allow_nan": False, "sort_keys": True}, +} + + +def _round_committed_floats(regression_dir: Path) -> None: + """ + Round floats in the committed JSON bundle to seven significant figures in place. + + Full-precision floats in ``series.json`` / ``diagnostic.json`` churn byte-for-byte + between CI and local runs even when numerically identical, + producing noisy diffs in the committed (git-tracked) bundle. + Rounding stabilises those bytes; + seven figures stays an order of magnitude under the regression compare tolerance (``rtol=1e-6``), + so a gate verdict is never flipped (see :mod:`climate_ref_core.regression._quantise`). + + Each file is re-serialised with the same JSON parameters used to write it natively, + so the only byte difference versus the copied file is reduced float precision. + A file is rewritten only when rounding actually changes its parsed content, + keeping float-free artefacts byte-identical to the copy. + The native blobs and their content-addressed digests are never touched; + this operates solely on the copied committed JSON. + + Parameters + ---------- + regression_dir + The test case ``regression/`` directory holding the committed bundle. + """ + for filename, dump_kwargs in _COMMITTED_FLOAT_JSON_KWARGS.items(): + path = regression_dir / filename + if not path.exists(): + continue + original = json.loads(path.read_text(encoding="utf-8")) + rounded = round_floats(original) + if rounded == original: + continue + path.write_text(json.dumps(rounded, **dump_kwargs), encoding="utf-8") # type: ignore[arg-type] + + def write_committed_bundle( source_dir: Path, regression_dir: Path, @@ -85,6 +129,10 @@ def write_committed_bundle( dest.unlink(missing_ok=True) to_placeholders(regression_dir, output_dir=output_dir, test_data_dir=test_data_dir) + # Round floats in place before digesting, + # so the committed bytes (and their recorded digests) are the stable, rounded ones. + # Placeholder substitution only rewrites path strings, so order relative to it does not matter for floats. + _round_committed_floats(regression_dir) return compute_committed_digests(regression_dir) diff --git a/packages/climate-ref-core/tests/unit/regression/test_capture.py b/packages/climate-ref-core/tests/unit/regression/test_capture.py index ce8c325f0..eabd001b5 100644 --- a/packages/climate-ref-core/tests/unit/regression/test_capture.py +++ b/packages/climate-ref-core/tests/unit/regression/test_capture.py @@ -59,6 +59,53 @@ def test_write_committed_bundle_sanitises_and_digests(tmp_path): assert digests["diagnostic.json"] == sha256_file(regression_dir / "diagnostic.json") +def _sig_figs(value: float) -> int: + """Count the significant figures in ``value``'s shortest round-trip repr.""" + # ``repr`` gives Python's shortest decimal that round-trips to the same float, + # so it reflects the value's true precision rather than padding it out. + text = repr(value) + mantissa = text.lstrip("-").split("e")[0] + digits = mantissa.replace(".", "").lstrip("0").rstrip("0") + return len(digits) if digits else 1 + + +def _assert_floats_rounded(obj, max_sig_figs=7): + """Recursively assert every float in ``obj`` has at most ``max_sig_figs`` figures.""" + if isinstance(obj, bool): + return + if isinstance(obj, float): + assert _sig_figs(obj) <= max_sig_figs, f"{obj!r} exceeds {max_sig_figs} sig figs" + elif isinstance(obj, dict): + for value in obj.values(): + _assert_floats_rounded(value, max_sig_figs) + elif isinstance(obj, list): + for item in obj: + _assert_floats_rounded(item, max_sig_figs) + + +def test_write_committed_bundle_rounds_floats(tmp_path): + output_dir = (tmp_path / "scratch" / "frag").resolve() + test_data_dir = (tmp_path / "test-data").resolve() + source = tmp_path / "scratch" / "frag" + source.mkdir(parents=True) + # Full-precision floats that round to fewer sig figs at write time. + source_diag = {"PROVENANCE": {"score": 1.843240715970751, "rmse": 2.813496471229112}} + (source / "diagnostic.json").write_text(json.dumps(source_diag)) + source_series = [{"dimensions": {"region": "global"}, "values": [1.761333624017425, 9.87654321]}] + (source / "series.json").write_text(json.dumps(source_series)) + regression_dir = tmp_path / "regression" + + write_committed_bundle(source, regression_dir, output_dir=output_dir, test_data_dir=test_data_dir) + + diag = json.loads((regression_dir / "diagnostic.json").read_text()) + series = json.loads((regression_dir / "series.json").read_text()) + _assert_floats_rounded(diag) + _assert_floats_rounded(series) + # The actual rounded values, not merely "<= 7 figures". + assert diag["PROVENANCE"]["score"] == 1.843241 + assert series[0]["values"] == [1.761334, 9.876543] + + def test_build_native_snapshot_digests_relpaths(tmp_path): base = tmp_path / "results" / "frag" base.mkdir(parents=True) diff --git a/packages/climate-ref-core/tests/unit/regression/test_quantise.py b/packages/climate-ref-core/tests/unit/regression/test_quantise.py new file mode 100644 index 000000000..5b926be7d --- /dev/null +++ b/packages/climate-ref-core/tests/unit/regression/test_quantise.py @@ -0,0 +1,118 @@ +"""Unit tests for :mod:`climate_ref_core.regression._quantise`.""" + +import math + +import pytest + +from climate_ref_core.regression._quantise import round_floats + + +@pytest.mark.parametrize( + "value, expected", + [ + (1.23456789, 1.234568), + (0.000123456789, 0.0001234568), + (123456789.0, 123456800.0), + (-1.23456789, -1.234568), + (3.14159265358979, 3.141593), + ], +) +def test_round_floats_seven_sig_figs(value, expected): + """A bare float is rounded to seven significant figures.""" + assert round_floats(value) == expected + + +def test_round_floats_is_idempotent(): + """Rounding an already-rounded value returns it unchanged.""" + once = round_floats(1.23456789) + twice = round_floats(once) + assert once == twice + + +@pytest.mark.parametrize("value", [0, 1, -42, 1_000_000_000_000]) +def test_round_floats_leaves_ints_untouched(value): + """Ints pass through unchanged and stay ints.""" + result = round_floats(value) + assert result == value + assert type(result) is int + + +@pytest.mark.parametrize("value", [True, False]) +def test_round_floats_leaves_bools_untouched(value): + """Bools are not treated as floats and stay bools (bool subclasses int).""" + result = round_floats(value) + assert result is value + assert type(result) is bool + + +@pytest.mark.parametrize("value", ["1.23456789", "hello", "", None]) +def test_round_floats_leaves_strings_and_none_untouched(value): + """Strings and None pass through unchanged.""" + assert round_floats(value) == value if value is not None else round_floats(value) is None + + +def test_round_floats_recurses_into_nested_dict(): + """Floats nested in dicts (incl. nested dicts) are rounded; other types preserved.""" + obj = { + "a": 1.23456789, + "b": {"c": 0.000123456789, "d": "text", "e": 7}, + "flag": True, + "n": None, + } + result = round_floats(obj) + assert result == { + "a": 1.234568, + "b": {"c": 0.0001234568, "d": "text", "e": 7}, + "flag": True, + "n": None, + } + assert type(result["flag"]) is bool + assert type(result["b"]["e"]) is int + + +def test_round_floats_recurses_into_lists(): + """Floats in lists (incl. nested lists) are rounded.""" + obj = [1.23456789, [0.000123456789, "x"], 5] + assert round_floats(obj) == [1.234568, [0.0001234568, "x"], 5] + + +def test_round_floats_handles_list_of_dicts(): + """series.json is a list of per-series dicts; each is rounded recursively.""" + obj = [ + {"values": [1.23456789, 2.345678912], "index": ["2000-01-16T12:00:00"]}, + {"values": [9.87654321], "index": ["2001-01-16T12:00:00"]}, + ] + assert round_floats(obj) == [ + {"values": [1.234568, 2.345679], "index": ["2000-01-16T12:00:00"]}, + {"values": [9.876543], "index": ["2001-01-16T12:00:00"]}, + ] + + +def test_round_floats_rounds_tuples_to_lists(): + """Tuples are walked recursively (returned as lists, matching JSON semantics).""" + assert round_floats((1.23456789, "x", 3)) == [1.234568, "x", 3] + + +def test_round_floats_custom_sig_figs(): + """The number of significant figures is configurable.""" + assert round_floats(1.23456789, sig_figs=3) == 1.23 + + +@pytest.mark.parametrize( + "value", + [ + 1.843240715970751, + 2.813496471229112, + 0.2389045018665123, + 123456789.987654, + 1.0e-12, + -9.99999999e10, + ], +) +def test_round_floats_stays_within_compare_tolerance(value): + """ + Rounding error stays an order of magnitude under the regression compare tolerance + (``rtol=1e-6``/``atol=1e-8``), so it can never flip a boundary gate verdict. + """ + rounded = round_floats(value) + assert math.isclose(value, rounded, rel_tol=1e-6, abs_tol=1e-8)