Skip to content

[PROBE: mass] Add time-origin invariance with aligned calendar shifts - #11

Open
SiavashShams wants to merge 1 commit into
Flood-Lab:mainfrom
SiavashShams:probe/time-origin-invariance
Open

[PROBE: mass] Add time-origin invariance with aligned calendar shifts#11
SiavashShams wants to merge 1 commit into
Flood-Lab:mainfrom
SiavashShams:probe/time-origin-invariance

Conversation

@SiavashShams

@SiavashShams SiavashShams commented Sep 8, 2026

Copy link
Copy Markdown

Closes #10

What this probe asserts

This adds mass/time-origin-invariance: the same generated weather and catchment are run from 2000-01-01 and 1972-01-01. The 4,015-day records preserve month, day, weekday, day of year and leap-day positions. Only the absolute calendar origin changes; forcing values, static attributes, initialization contract and model seed stay the same.

For each required flux or storage, the maximum pointwise difference after spinup must satisfy max(abs(shifted - control)) / max(mean(abs(control)), 1) <= 1e-9. The one-unit floor handles empty stores. Both variants must independently close their water budgets within 5% of input precipitation. A separate control-run guard rejects zero-runoff or constant-flux outputs.

Two small criterion changes support the probe: an opt-in normalization_floor for invariance, preserving the existing default, and paired_closure, which applies the existing closure rule to each named variant. Invariance also rejects nonfinite inputs or arithmetic overflow that could otherwise produce a false pass.

The README explains the stationary, rain-only scope. This does not assess snowmelt behavior or changing physical drivers, and a forcing-responsive fixed partition can still pass. The strict tolerance is tested on deterministic CPU references.

Discrimination

The acceptance gate passes with all three gate seeds.

Reference model Expected Criterion that catches it
reference_bucket PASS n/a
flex_lumped PASS n/a
flex_topo PASS n/a
sacsma_snow17 PASS n/a
reference_calendar FAIL invariance
reference_degenerate FAIL non_degenerate
reference_leaky FAIL paired_closure

On five additional seeds (20261001 through 20261005), all 20 physical-model evaluations pass with exactly zero invariance deviation. All five calendar-model evaluations fail only invariance while both water budgets pass. Its smallest normalized deviation is 22.672 against the 1e-9 limit; the largest physical-model closure residual is 1.619e-15 against 0.05.

Validation

  • ht validate: passes for all 15 probes and 23 models.
  • pytest -q: 174 tests pass, including calendar alignment, identical staged metadata and seeds, repeatability, shorter event windows, both closure budgets and numeric edge cases.
  • ht gate: all 15 probes separate the declared reference models.
  • Separate 25-evaluation validation batch passes the expectations above.

Checklist

  • There is an accepted proposal issue and this PR closes it.
  • ht validate passes.
  • ht gate --probe mass/time-origin-invariance passes.
  • The generator is deterministic given a seed and commits no data.
  • Under-one-minute performance on a two-core runner is still to be verified. The separate 25-evaluation batch took under 5 seconds locally on macOS with Python 3.12.
  • Tolerance and denominator are justified in probe.yaml.
  • The existing closure-conserving reference_calendar fails the new invariance check, isolating a fault that closure alone misses.

@SiavashShams
SiavashShams marked this pull request as ready for review September 8, 2026 19:04
@chrimerss

Copy link
Copy Markdown
Contributor

Thanks @SiavashShams , this is a careful piece of work. The probe itself is right: the 28-year shift keeps every calendar feature aligned over this window (I checked the leap days and the 1,461-week arithmetic), the forcing is byte-identical between variants, and reference_calendar is caught by exactly the criterion it was built to be caught by. Gate, validate and the full suite all pass for me locally. I'd like one round of revision before merging, mostly to make the PR smaller.

Requested changes

1. Drop paired_closure and use plain closure with reference_leaky: closure.
On this probe invariance already requires every closure input (evspsbl, mrro, mrso, snw, canopy, and the optional gw, channel, gwex) to match between variants to 1e-9 on identical forcing. So paired_closure can only fail where closure on the control also fails, and reference_leaky leaks 15% of rain on every step regardless of calendar, so it trips plain closure too. That removes the wrapper, tests/test_paired_closure.py, and three smaller issues that come with it (the variants default is wrong under variant_selection: native_and_finer, max() over a NaN residual is order-dependent, and report.py only recognises a criterion literally named closure, so the archive would record all criteria pass instead of the residual). If a future probe with genuinely different variants needs per-variant budgets, the right place is a general variant selector in evaluate_criteria, and I'd rather take that as its own PR.

2. Drop normalization_floor, or at least don't ship it here.
I ran the gate with the floor removed: all four physical models replay bit-identically (max deviation exactly 0.0 on every variable including snw), and the must_fail set trips the same criteria. So no baseline exercises it. If we do want an absolute tolerance later, atol in the numpy atol + rtol*scale form says it directly rather than as a product of two knobs, and it would deserve its own discussion since the module's premise is that invariance has no tolerance to tune towards.

3. Don't turn non-finite output into a harness ERROR.
The new _deviation raises on NaN/inf, which run_probe reports as verdict FAIL, reason ERROR with an empty scorecard. On main, inf was a scored VIOLATION on invariance with the other criteria still recorded; on this branch it is indistinguishable from a broken adapter, ht run exits 2 (which the model-eval workflow treats as a harness failure), and a must_fail entry can never name it. The NaN half is an improvement over the old silent pass, so keep the check, but return a FAIL CriterionResult that names the variable and variant rather than raising. The overflow-of-finite-inputs case can stay an exception if you want, though I doubt any real model reaches it.

4. max_runtime_s: 60300.
The budget is per run. With min_window_days: 365 a submitted Docker model gets 730 rows per variant, about twice what catchment-closure runs under its 60 s, and google_flood_forecast already needs ~19 s for 395 rows on a fast machine. Every other paired or year-long probe uses 300; I'd match area-invariance here unless you've timed a container on a two-core runner (the checklist item is still unchecked).

Small things

  • test_event_window_preserves_pairing_and_annual_guard never looks at non_degenerate diagnostics, so it passes even when the annual runoff-ratio check is skipped. Asserting "runoff_ratio_check" not in diagnostics for reference_bucket would make it test what its name says.
  • The README probe table and counts, the reference_calendar row ("waiting for an invariance probe"), ROADMAP.md (still "unclaimed"), site/index.html (still "wanted" in three languages), and CONTRIBUTORS.md need updating. I can do these on merge if you'd prefer, just say so.
  • period_years: 10 is fine, but a sentence in the README on why ten rather than the three the sibling generator uses would help the next reader.

Once 1 to 3 land this is a three-file probe plus its tests, which is what I'd hoped for. Happy to talk through any of these on the issue if you disagree with the reasoning on (1) in particular.

@chrimerss chrimerss added revision revision is needed before merge probe-proposal A proposed new probe labels Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

probe-proposal A proposed new probe revision revision is needed before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PROBE] mass/time-origin-invariance: paired calendar shifts

2 participants