Skip to content

Expand snm coverage for packaged snow models - #153

Open
jinlong-hu wants to merge 17 commits into
Flood-Lab:mainfrom
jinlong-hu:enhancement/snm-coverage
Open

jinlong-hu wants to merge 17 commits into
Flood-Lab:mainfrom
jinlong-hu:enhancement/snm-coverage

Conversation

@jinlong-hu

@jinlong-hu jinlong-hu commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Closes #137

Summary

This PR expands snm coverage for packaged models with an explicit snow module, so that mass/snowpack-mass-closure can be evaluated where a model-native snow-module outflow is available.

The mappings added are:

Model snm mapping
CWatM Rain + SnowMelt + IceMelt
δHBV2 RAIN + tosoil
LISFLOOD Rain + SnowMelt
Wflow SBM native snow.runoff
SUMMA native scalarRainPlusMelt

The other relevant packaged models were also reviewed. flex_lumped and flex_topo do not expose a separate snow-module control volume with a distinct liquid outflow matching the snm contract. google_flood_forecast has no snow module, while modflow6 represents groundwater and surface-water exchange without snow processes, so they remain unchanged.

Implementation

For each supported model, this PR:

  • maps the model-native snow-module outflow to snm;
  • updates model.yaml to declare the new flux;
  • documents the physical interpretation of the mapping;
  • preserves the native model quantity rather than reconstructing snm from a residual;
  • updates the archived probe results and model standings.

For δHBV2, snm is RAIN + tosoil. tosoil is read directly from the model, while RAIN is reconstructed using the same temperature-threshold rule as the pinned hydrodl2 version. Each adapter run cross-checks that reconstruction against the model's exposed snow stores using Δ(SNOWPACK + MELTWATER) = pr - snm. A 1e-4 mm / four-float32-ULP threshold is retained as a precision diagnostic, while the adapter stops only when the residual exceeds max(1e-3 mm, 64 ULP). This keeps the check sensitive to an incorrect rain partition while avoiding failures caused by native float32 roundoff at deep snowpacks. On the collaborator-provided 730-day deep-snow reproduction, the maximum step residual is 0.000195183 mm against a four-ULP diagnostic threshold of 0.00048828125 mm, and the adapter writes all 730 output rows.

For Wflow SBM, snm is read directly from the native snow.runoff variable. Its snowpack budget closes to machine precision. In the snowpack-closure probe canopy_capacity_mm = 0, mapped to Cmax = 0, so precipitation reaches the snow module without upstream interception.

For SUMMA, snm is mapped to scalarRainPlusMelt. With explicit snow layers this is the basal liquid flux leaving the snowpack; without an explicit snow layer it also carries rain and melt entering the soil. The adapter also reports signed sbl = -(scalarSnowSublimation + scalarCanopySublimation), preserving SUMMA's native net ice-vapour exchange from both snowpack and canopy ice: positive values represent net sublimation and negative values represent net frost deposition.

Results

After rebasing onto the current main and incorporating the review fixes, I reran the adapter checks and the full gate-seed suite.

Model Standing Snowpack closure Spin-up cycle invariance
CWatM 18 / 22 PASS PASS
δHBV2 12 / 22 PASS FAIL
LISFLOOD 20 / 22 PASS PASS
Wflow SBM 19 / 22 PASS PASS
SUMMA 10 / 25 PASS PASS

The δHBV2 spin-up failure is the existing mrso state-bound issue: the reported soil moisture exceeds the declared capacity under all three tested spin-up lengths.

SUMMA now passes mass/snowpack-mass-closure, with a cumulative residual of 0.0000% of precipitation. The closure uses independent native quantities: scalarRainPlusMelt as snm, signed net ice-vapour exchange as sbl, and scalarSWE as snw; it is not reconstructed from a residual. The snowpack-closure probe has no canopy interception, so sbl there is the snowpack exchange alone.

The sbl contract now defines the variable as signed net sublimation minus frost deposition from every reported ice store, including snowpack and canopy ice. With SUMMA's pack and canopy sublimation both included, every step that reports sublimation or deposition satisfies the latent-heat ice-vapour equality within tolerance. The remaining latent-heat failures are instead caused by SUMMA's liquid evaporation being converted with its constant LH_vap; no verdict changes, and SUMMA remains at 10/25.

The LISFLOOD .6 archive was generated on a native x86-64 host with a 13th Gen Intel(R) Core(TM) i5-13400F. Its ten-year adapter invocations took 12.95--14.65 s for mass/precipitation-counterfactual and 14.14--14.97 s for mass/human-abstraction, all within the probes' 60 s per-invocation runtime budgets.

Validation

All five modified adapters pass ht verify-adapter.

The repository-level checks also pass:

pytest -q
1274 passed, 8 skipped

ht validate
✅ validation passed: 35 probe(s), 80 model(s), suite 0.1.0

ht gate
✅ GATE PASSED: every probe separates the reference models as declared.

python scripts/site_standings.py --check
site/index.html standings match models/result.csv

The archived rows for these model versions were regenerated against the current 35-probe suite, including mass/groundwater-datum-invariance, mass/spinup-cycle-invariance, and momentum/froude-regime.

@chrimerss

Copy link
Copy Markdown
Contributor

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by Claude (changes-suggested), not a maintainer approval.

The PR maps snm (snow-module liquid outflow) for five packaged models so mass/snowpack-mass-closure can score them, bumps each model version, and archives a full 33-probe gate-seed row set per model, updating the top-level README, each model README and the site's three language blocks.

The mappings themselves look sound. CWatM (Rain + SnowMelt + IceMelt), LISFLOOD (Rain + SnowMelt) and Wflow SBM (native snow.runoff) are read from native terms and close the pack budget algebraically; the Julia column re-index at WflowSbmAdapter.jl:714-722 and the LISFLOOD record unpacking are both consistent. The archived standings (12/22, 19/22, 9/25, 18/22, 20/22) match models/result.csv exactly, and every new version has a row for all 33 probes, so tests/test_docs_in_sync.py should pass.

The thing to decide before merging is SUMMA. Its new snm row is a FAIL at 18.85% of precipitation, and the README explains it as a contract limitation: sbl cannot be reported because HydroTuring's sbl must be non-negative. Nothing in AGENTS.md, spec.py or the criteria says that — coherence.py explicitly nets out deposition reported in sbl. So this publishes a snowpack conservation violation against SUMMA that is most likely an adapter omission, not the model.

Two secondary items: the LISFLOOD README now contradicts itself about the two ten-year probes (ERROR vs completed), and δHBV2's reconstructed RAIN term is not exercised by any archived case.


models/lisflood/README.md:389 medium: LISFLOOD README still documents ERROR rows the archive no longer has

The rewritten Result section (line 398) says "FAIL (VIOLATION), 20 of 22 probes passed" and models/result.csv:762-794 shows mass/precipitation-counterfactual PASS and mass/human-abstraction VIOLATION, both on 3650-day windows. But lines 389-394 still read "A ten-year daily record takes 97 s there. That is over the 60 s budget of mass/precipitation-counterfactual and mass/human-abstraction … so the archived ERROR rows record no wall time of their own", and lines 434 and 450 still say those probes are ERROR. The "Native re-run" section that explained how the provisional ERROR rows would be replaced was deleted in this PR, and model.yaml's closing comment (which also carried the "Default flood-event window" note) went with it.

Nothing in the adapter changed its speed and both probes still carry max_runtime_s: 60 (probes/mass/precipitation-counterfactual/probe.yaml:35, probes/mass/human-abstraction/probe.yaml:35), so the verdict moved from FAIL (ERROR) to FAIL (VIOLATION) purely because a different host ran it — and the README no longer records which host produced the archived rows.

Failure scenario: a reader checking why LISFLOOD's verdict changed finds the README asserting the two probes cannot finish inside their budget on the host, and no statement of the host these rows came from; the next person to re-run gets different rows and cannot tell whether that is a regression.

Suggested fix: rewrite lines 378-394 for the host that produced the .6 rows (or say plainly that the archive is now native x86-64 and the emulated timings are historical), and restore the window sentence to model.yaml.

Posted by the pr-review workflow. Run

Comment thread models/summa/README.md Outdated
Comment thread models/dhbv2/ht_adapter.py
Comment thread models/lisflood/README.md Outdated
Comment thread models/wflow_sbm/src/WflowSbmAdapter.jl
Comment thread models/wflow_sbm/README.md
Comment thread models/result.csv Outdated
Comment thread README.md Outdated
@cehw

cehw commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Thanks, Jinlong — exposing the native snow-module outflow makes the existing snow-balance probe usable for more models. δHBV2’s mapping matched the native model in my cold-snow and rain-on-snow runs.

One issue to fix before merging: the new 1e-4 mm cross-check at 21a8308 stops δHBV2 on a 730-day synthetic seasonal case with peak snow water equivalent of 1,456 mm. The residual is 0.000199 mm, comparable to native float32 roundoff. With the same forcing and published ep100 weights, the previous adapter writes all 730 rows; the new one writes none.

Could the check account for native state precision while retaining the rain-reconstruction check? I ran this on Linux with the pinned model dependencies, using Python 3.10 outside Docker, and have included the reproduction input below. That would keep this added coverage usable for deeper snowpacks too.

Reproduction input and environment

Native CPU inference on Linux: Python 3.10.19, torch 2.5.1+cpu, numpy 1.26.4, pandas 2.3.3, dmg 1.4.3, hydrodl2 1.3.5, dhbv2 0.5.4; DHBV_THREADS=2 and OMP_NUM_THREADS=2. I used the published hbv_2_ep100.pt, config.yaml and normalization_statistics.json from the same official release bundle named in the Dockerfile. This was not a Docker-image test.

The input generator below is extracted from the diagnostic I ran. Its regenerated forcing and static parameters matched the recorded experiment exactly. It produces two synthetic seasons, with 180 cold days, 120 dry melt days and 65 warm days per season. Total precipitation is 2,973.091151 mm, with a maximum daily value of 81.878663 mm. No explicit elevation is supplied, so the adapter uses its training-mean elevation.

Save as reproduce_input.py in a scratch directory and run it with the stated numpy/pandas versions:

import json
from pathlib import Path
import numpy as np
import pandas as pd

STATIC = {
    "area_km2": 250.0,
    "soil_capacity_mm": 320.0,
    "canopy_capacity_mm": 0.0,
    "degree_day_factor_mm_per_C_day": 3.2,
    "baseflow_coefficient": 0.006,
    "snow_threshold_degC": 0.0,
    "latitude_deg": 44.0,
}

def frame(time, pr, tas):
    doy = pd.to_datetime(pd.Series(time)).dt.dayofyear.to_numpy()
    daylength = 1.0 + 0.35 * np.cos(2 * np.pi * (doy - 172) / 365)
    pet = np.maximum(0.0, 0.13 * (tas + 5.0)) * daylength  # the probe generator's PET form
    return pd.DataFrame({"time": list(time), "pr": np.round(pr, 6), "tas": np.round(tas, 6), "pet": np.round(pet, 6)})

def deep_snow_case(scale, years=2, seed=20261002):
    """Single deep winter per year: 180 cold days (all snow), 120 dry melt days, 65 warm days.
    Same draws at every scale, so only the amounts (and the SWE magnitude) change."""
    rng = np.random.default_rng(seed)
    n = 365 * years
    time = pd.date_range("1999-10-01", periods=n, freq="D").strftime("%Y-%m-%d")
    day = np.arange(n) % 365
    cold, melt = day < 180, (day >= 180) & (day < 300)
    noise, innov = np.zeros(n), rng.normal(0.0, 0.8, n)
    for t in range(1, n):
        noise[t] = 0.72 * noise[t - 1] + innov[t]
    tas = np.where(cold, np.minimum(-6 + noise, -3.0), np.where(melt, np.maximum(8 + noise, 4.0), 10 + noise))
    snow_draw, wet_cold = rng.gamma(0.8, 1.0, n), rng.random(n) < 0.5
    rain_draw, wet_late = rng.gamma(0.7, 8.0, n), rng.random(n) < 0.3
    pr = np.where(cold & wet_cold, scale * snow_draw, 0.0)
    pr = np.where(~cold & ~melt & wet_late, rain_draw, pr)
    return frame(time, pr, tas), dict(STATIC)

case = Path(__file__).resolve().parent / "publication_input"
(case / "input").mkdir(parents=True, exist_ok=True)
(case / "output").mkdir(exist_ok=True)
f, static = deep_snow_case(scale=18.0, years=2, seed=20261002)
f.to_csv(case / "input/forcing.csv", index=False)
(case / "input/static.json").write_text(json.dumps(static, indent=2))
request = {
    "case_id": "deep-snow-native-control", "seed": 20261002,
    "timestep": "PT1D", "n_steps": len(f),
    "input": {"forcing": "input/forcing.csv", "static": "input/static.json"},
    "output": {"table": "output/result.csv", "run": "output/run.json"},
}
(case / "request.json").write_text(json.dumps(request, indent=2))
print(case / "request.json")

The paired model runs used the base adapter, version .3 and the PR adapter, version .4, with the same model directory and input. To repeat the comparison, save these as ht_adapter_base.py and ht_adapter_pr.py, set DHBV_MODEL_DIR to the unpacked bundle directory, and create separate case copies before either run:

export DHBV_THREADS=2 OMP_NUM_THREADS=2
cp -R publication_input baseline_case
cp -R publication_input pr_case
python ht_adapter_base.py --request baseline_case/request.json
python ht_adapter_pr.py --request pr_case/request.json

Observed: base exit 0 and 730 output rows; PR exit 1 before writing the output table or run metadata. The error ends with:

RuntimeError: reconstructed HBV rain partition is inconsistent with SNOWPACK + MELTWATER: maximum step residual 0.000198998 mm

At the worst step, float32 spacing is 0.00012207 mm; the full-record signed snow-budget residual is only -1.65×10⁻⁸ of precipitation. The base snow-storage series matches the native states observed in the PR diagnostic exactly.

@jinlong-hu
jinlong-hu force-pushed the enhancement/snm-coverage branch from 21a8308 to faab065 Compare October 1, 2026 13:37
@jinlong-hu

Copy link
Copy Markdown
Contributor Author

@cehw Thanks for the detailed reproduction. I reproduced the deep-snow case and updated the δHBV2 cross-check to account for the native float32 precision while retaining the rain-reconstruction validation.

The check now keeps the existing 1e-4 mm absolute floor and, where necessary, expands the per-step tolerance to four float32 ULPs at the native snow-state/flux scale.

On your 730-day reproduction case, the updated adapter completes successfully with:

  • maximum step residual: 0.00019518285989761353 mm
  • maximum precision-aware tolerance: 0.00048828125 mm

The full output contains all 730 rows.

I also re-ran:

  • ht verify-adapter --model dhbv2 — PASS
  • ht run --model dhbv2 --probe mass/snowpack-mass-closure --gate-seeds — PASS
  • the full δHBV2 suite — unchanged at 12/22, with snowpack closure PASS
  • repository tests — 1259 passed, 8 skipped
  • ht validate — 34 probes / 77 models
  • ht gate — PASS

The branch has also been rebased onto the current main, and the five modified model archives have been regenerated on the current 34-probe suite.

@jinlong-hu
jinlong-hu force-pushed the enhancement/snm-coverage branch from faab065 to 29e53da Compare October 2, 2026 13:21
@cehw

cehw commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the fix, Jinlong — the precision-aware tolerance keeps this check useful for deeper snowpacks. I re-ran 29e53da on Linux with the same official weights: both cases complete, and deliberately omitting rain from snm still triggers the check before output.

This resolves my δHBV2 finding. My checks cover that adapter’s native inference outside Docker.

@chrimerss chrimerss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Jinlong. Reviewed at 29e53da.

What I checked. On the PR head: pytest 1279 passed, ht validate (35 probes, 80 models) and ht gate pass, and CI is green. Each mapping matches the model's own source:

  • CWatM: snow_frost.py:777-836 splits melt into SnowMelt and IceMelt, and interception.py:119 takes both downstream. Rain + SnowMelt + IceMelt counts nothing twice.
  • LISFLOOD: snow.py:183-188 folds ice and glacier melt into SnowMelt, so Rain + SnowMelt is right.
  • δHBV2: hydrodl2 1.3.5 hbv_2.py:474-511 gives the same parTT / 2000 m partition, and (dif_count, nmul) is the static layout snow_module_outflow assumes.
  • Wflow: snow.runoff is mm Δt⁻¹ (snow.jl:13), so dividing by dt_days is right. The snow module takes throughfall plus stemflow (snow.jl:149), which equals pr only because this probe sets canopy_capacity_mm = 0. The README says so.

I rebuilt the five images and re-ran on an arm64 Docker host. All five mass/snowpack-mass-closure rows and SUMMA's two energy rows reproduce exactly. I also re-ran δHBV2's full gate-seed suite: 12/22, no ERROR, and the new snow check never fired on the 35 probes. 34 of the 35 rows are byte-identical. mass/area-invariance differs only in the last digit (2.938e+01 vs 2.939e+01), a platform float difference.

One limit on what these PASS rows prove: this probe's case only has precipitation at or below −3 °C, so the Rain term in each new snm never carries water in an archived case. A mapping that left out rain would also pass. δHBV2's runtime check covers its own rain term on every other probe. Nothing exercises rain for the other four.

SUMMA sbl blocks the merge; the rest is ready.


1. SUMMA's signed, snow-only sbl undoes the .4 decision and turns its energy failures into adapter failures (models/summa/ht_adapter.py:568)

.4 (dc95382, from #43's review) dropped sbl because flux_identity reads it as a non-negative share of evspsbl (coherence.py:147-157), and SUMMA's sublimation goes negative on deposition. .5 brings back a signed sbl and also drops the canopy term .3 had. The effect, from the gate-seed output on all five latent-heat seeds:

  • Both energy rows now lead with an adapter fault: 'sbl' is negative on 146 steps (latent-heat) and 51 steps (partition). Before, they led with SUMMA's own latent-heat mismatch.
  • With the canopy term out, canopy-ice sublimation gets charged as liquid. The worst step on every seed has snow on the ground, 7–9 mm of canopy ice and air just above 0 °C. Nearly all of the evaporation is canopy sublimation, and SUMMA converts it correctly at λs. Seed 788749541, step 411: E 3.17 mm, sbl 0.001 mm, hfls 103.87 W m⁻², and λs·E is 103.87. Violating steps go from 931–962 to 1274–1329 per seed, and the worst step from 7.3–9.4× to 23.6–23.75× the tolerance.
  • The README misreads this:
    • README.md:498-503 puts the 23.75× on SUMMA's constant LH_vap. It comes from canopy ice reported as liquid.
    • README.md:662-666 says the change "follows the contract's support for signed snow-atmosphere exchange". AGENTS.md:168 gives no such support. It defines sbl as "the sublimating share of evspsbl" without a sign rule, and the only criterion that checks sign forbids negatives.

snm can't go in without sbl. With sbl dropped, SUMMA's snowpack closure fails on all five seeds at 12.0–18.8% of precipitation. Clipped at zero it passes (0.06–0.09%), but clipping misreports deposition, which is why .4 turned it down.

This is a contract question, not just an adapter one. Two parts:

  • (a) Should sbl be the signed net ice-vapour exchange? flux_identity's equality λv(E − sbl) + λs·sbl still holds physically for deposition.
  • (b) Is sbl all the ice that left (AGENTS.md, flux_identity), or only the pack's (snowpack closure, melt_energy's M = −Δ(snw − lwsnl) − Σsbl)? A model with canopy ice cannot meet both.

Suggested fix, which I tried on the gate seeds:

  • Adapter (this PR): sbl = -(scalarSnowSublimation + scalarCanopySublimation), signed, as .3 had it. This is every kilogram that left as ice, which is what AGENTS.md:168 asks for.
    • The snowpack closure is unaffected. That probe sets canopy_capacity_mm = 0, so the adapter runs SUMMA bare and it still closes at 0.0000% on all five seeds.
    • On latent-heat, every sublimating step now falls within tolerance (worst 0.81–0.85×; 0.86 on partition). What still fails is liquid evaporation converted at SUMMA's constant LH_vap, on 966–1013 steps per seed, worst 4.95–5.64×. That failure belongs to the model.
    • flux_identity still flags two things the contract has to settle: negative sbl (152–209 steps per seed), and canopy-ice sublimation on days with no snow (36 steps on the worst seed).
  • Contract (its own PR, energy pool):
    • AGENTS.md: define sbl as the signed net ice–vapour exchange, sublimation minus deposition, from any ice store, positive away from the surface.
    • coherence.py, flux_identity:
      • Remove the subl < 0 check. The equality λv(E − sbl) + λs·sbl still holds for deposition.
      • Count the reported canopy as somewhere ice can be. I tested this by adding canopy to snw: the no-ice failures go away on every seed.
      • Apply the no-ice check to |sbl|, so a model can't claim fictitious deposition to pull its λ down.
    • energy/snowmelt-energy-water: melt_energy subtracts Σsbl from the pack's ice, but that probe sets canopy_capacity_mm = 2.0. Set it to 0, as the snowpack probe does, so the snow-only and all-ice readings agree. SUMMA is N/A there today, so nothing moves.

Merge the contract PR first and archive SUMMA's .5 rows after it. If you'd rather not wait, take SUMMA out of this PR: it stays at .4, N/A on snowpack, energy rows unchanged. The other four models don't depend on this. CWatM's sbl (snowEvap) predates this PR and is never negative.

2. Site still says LISFLOOD FAIL (ERROR) (site/index.html:868, 1112, 1356)

The archive and the README now say FAIL (VIOLATION), 20/22. All three language rows still have the FAIL (ERROR) / FALLA (ERROR) / 未通过(错误) tag. tests/test_docs_in_sync.py:114-125 checks only the N/M, so CI misses it. Fix: retag as violation in all three.

3. LISFLOOD README count and host (models/lisflood/README.md:398)

"11 N/A (9 INCOMPLETE, 2 INCOMPATIBLE)": the .6 rows have 13 N/A, 11 of them INCOMPLETE. The list leaves out mass/groundwater-datum-invariance and momentum/froude-regime. The README also no longer says which host produced the .6 rows. The two ten-year probes score only on a host fast enough for their 60 s budget, and under amd64 emulation they took 87–97 s (your README), so they go to ERROR there. Please say which host made the .6 rows (for example: native x86-64, the CPU, and the time per ten-year case), so the next re-run can tell a regression from a slower machine.

4. δHBV2's self-check raises on a 4-ULP bound (models/dhbv2/ht_adapter.py:417, 434)

The fault it guards against, rain put in the wrong partition, moves whole millimetres. The bound is four float32 ULPs at the snow scale, and cehw's deep-snow case already reaches 1.6 of them. If roundoff on some other platform or step goes over four, the adapter exits 1 and the whole case is archived as FAIL (ERROR) for a precision artefact. Suggest keeping 4 ULP as the recorded diagnostic and raising only well above roundoff, for example max(1e-3 mm, 64 ULP). That still catches any misclassified rain above about 0.01 mm at 1,456 mm SWE. Not blocking.

AGENTS.md now defines `sbl` as sublimation minus frost deposition, from the
snowpack and canopy ice alike, negative on a step of net deposition. Before,
it was "the sublimating share of `evspsbl`" with no sign rule, while
flux_identity refused a negative share, so a model that deposits frost had
no honest way to report it.

flux_identity:
- drops the `sbl < 0` check. Deposition releases lambda_s per kilogram, so
  LE = lambda_v (E - sbl) + lambda_s sbl holds for it as written;
- counts a reported `canopy` holding water within 2 degC of the snow
  threshold as somewhere ice can be (`other_ice_stores`,
  `other_ice_margin_degC`). On the gate seeds SUMMA's canopy-ice
  sublimation with no snow on the ground falls at or below 0.07 degC;
- applies the no-ice check to |sbl|, so fictitious deposition on a warm day
  is refused like a fictitious sublimating share.

Three tests pin it; all three fail on the previous criterion.

energy/snowmelt-energy-water sets canopy_capacity_mm = 0 (version 2), so
melt_energy's M = -d(snw - lwsnl) - sum(sbl dt) reads the pack's own
sublimation, as mass/snowpack-mass-closure already does. Over 200 seeds the
three reference models' verdicts and melt_energy values are identical at
2.0 and 0.0: none of them carried canopy water through the block.
`sbl` is now -(scalarSnowSublimation + scalarCanopySublimation), the signed
form `.3` had. SUMMA converts both at LH_sub; with the canopy term left out,
canopy ice sublimating over a pack was declared liquid, and that alone made
the worst latent-heat step 23.6-23.75x the tolerance on every gate seed.

On mass/snowpack-mass-closure the case has no canopy, so the adapter runs
SUMMA bare and `sbl` is the pack's own: still 0.0000% on all five seeds.
On the two energy probes every step reporting sublimation or deposition now
meets the equality (worst 0.81-0.86x); what still fails is the liquid share
at SUMMA's constant LH_vap, 966-1013 of 3650 days a seed, worst 5.64x, as
.4 failed without `sbl`.

The four SUMMA .5 rows that read `sbl` are re-run on the gate seeds
(latent-heat, evaporative-partition, snowmelt-energy-water, snowpack); no
verdict moves and SUMMA stays at 10 of 25.
The archive and both READMEs say FAIL (VIOLATION), 20 of 22, but the three
site rows still carried the FAIL (ERROR) tag; the docs test checks only the
N/M. The LISFLOOD README counted 11 N/A where the .6 rows have 13, missing
mass/groundwater-datum-invariance and momentum/froude-regime.
The check raised at four float32 ULPs, and cehw's deep-snow case already
reaches 1.6 of them, so roundoff on another platform could archive a case
as FAIL (ERROR). The fault it guards against, rain on the wrong side of the
partition, moves whole millimetres. Steps over four ULPs are now counted in
run.json, and the adapter stops above max(1e-3 mm, 64 ULP), which still
catches any misclassified rain above about 0.01 mm at a 1,456 mm pack.
Output is unchanged, so there is no version bump; the snowpack row
reproduces byte for byte.
@chrimerss

Copy link
Copy Markdown
Contributor

@jinlong-hu I pushed four commits to this branch (29e53da..2534648) that do what my review asked. Before pushing: pytest 1282 passed, and ht validate, scripts/site_standings.py --check and the full ht gate pass.

  • 10c6725 sbl contract.
    • AGENTS.md now defines sbl as net sublimation minus frost deposition, from every ice store (pack and canopy), negative on net deposition.
    • flux_identity no longer rejects a negative sbl. Deposition releases λs, so the equality holds as written.
    • It now counts a reported canopy holding water within 2 °C of the snow threshold as ice. On the gate seeds, SUMMA's canopy-ice sublimation with no snow falls at or below 0.07 °C.
    • The no-ice check now applies to |sbl|, so fictitious deposition on a warm day is refused too.
    • Three new tests in tests/test_coherence.py pin this; all three fail on the previous criterion.
    • energy/snowmelt-energy-water now sets canopy_capacity_mm = 0 (version 2), so melt_energy reads the pack's own sublimation. Over 200 seeds the three reference models' verdicts and melt_energy values are identical at 2.0 and 0.0.
  • 3831ede SUMMA.
    • sbl = -(scalarSnowSublimation + scalarCanopySublimation).
    • I re-ran the four .5 rows that read sbl. The snowpack closure is still 0.0000% on all five seeds; that case has no canopy, so sbl there is the pack's alone.
    • Latent-heat now leads with SUMMA's own mismatch, the liquid share at constant LH_vap: 966–1013 of 3650 days a seed, worst 5.64×. Every step that reports sublimation or deposition meets the equality (worst 0.81–0.86×).
    • No verdict moves; SUMMA stays at 10/25. I rewrote the README's sbl passages to match.
  • 335fbb0 LISFLOOD. The three site rows now carry the FAIL (VIOLATION) tag, and the README counts 13 N/A.
  • 2534648 δHBV2. The snow check counts steps over 4 ULP in run.json and stops only above max(1e-3 mm, 64 ULP). Output is unchanged, so there is no version bump, and the snowpack row reproduces byte for byte.

One thing I left for you: in models/lisflood/README.md, please name the host that produced the .6 rows (for example native x86-64, the CPU, and the time per ten-year case). The two ten-year probes only score on a host fast enough for their 60 s budget.

@licm13 @cehw @xinlan-technology: 10c6725 changes the energy contract and flux_identity, so it needs two approvals from the energy pool. One open question for you: I did not add the mirror of the "larger than the evaporation" bound (sbl ≥ min(E, 0)). On one gate seed SUMMA deposits 0.0028 mm more frost than its net condensation while liquid evaporates elsewhere in the same step, so that bound would refuse a physically possible step.

@jinlong-hu

Copy link
Copy Markdown
Contributor Author

@chrimerss Thanks — I’ve added the requested LISFLOOD host/runtime documentation.

The .6 rows were reproduced on a native x86-64 host with a 13th Gen Intel Core i5-13400F. The ten-year adapter invocations took 12.95–14.65 s for mass/precipitation-counterfactual and 14.14–14.97 s for mass/human-abstraction, all within the 60 s per-invocation budgets.

I also re-ran the repository checks after syncing your four commits:

  • pytest tests/test_docs_in_sync.py -q — 241 passed
  • pytest -q — 1274 passed, 8 skipped
  • ht validate — 35 probes / 80 models
  • ht gate — PASS
  • scripts/site_standings.py --check — PASS

…d for

10c6725 dropped the check that refused a negative sbl and put no lower bound
in its place. Any latent heat below lambda_s E is then matched by
sbl = (LE - lambda_v E) / (lambda_s - lambda_v): a model reporting half the
latent heat its evaporation needs on every pack step passed with that
deposition reported, and failed without sbl. Review found it.

- sbl may not deposit more than the condensation it is a share of, with
  0.01 mm/day of slack (deposition_allowance_mm_per_day), which buys at most
  0.04 W m-2. With both bounds a split moves the required latent heat only
  inside the interval a model without sbl is held to. SUMMA's largest
  excess on the gate seeds is 0.0028 mm/day; it trips nothing.
- The detail and diagnostics report deposition steps beside sublimation
  steps, so a failure on a frost step is visible.
- other_ice_stores must be a list; a bare string was split into letters and
  silently disabled the canopy route.
- The canopy margin narrows from 2 to 0.5 degC, as review suggested: SUMMA's
  canopy-ice sublimation with no snow falls at or below 0.07 degC, and its
  flux_identity results are identical at 2.0 and 0.5 on all 11 runs.
- Four tests: the solved-deposition case, the 0.5 degC canopy margin at
  +0.3 and +1.0 degC, and a canopy that empties during the step. Setting the
  margin to 0 or 2, dropping the start-of-step canopy, or disabling the
  bound each turns exactly one test red.

SUMMA's two energy rows are re-run for the new detail text; no verdict moves.
Its README no longer says frost steps meet the equality: they fail on the
liquid evaporating beside the frost, at 2.42-3.92x on latent-heat. The
latent-heat README, spec.py and SUMMA's .4 history are brought into line.
@chrimerss

Copy link
Copy Markdown
Contributor

@licm13 @cehw @xinlan-technology I pushed 4388ee2 on top of Jinlong's 4ba2178. It fixes a hole my 10c6725 opened in flux_identity, and narrows the canopy margin. Please review both with the rest of the sbl contract change.

The hole. 10c6725 dropped the check that refused a negative sbl and put no lower bound in its place. Any latent heat below λs·E could then be matched by reporting sbl = (LE − λv·E)/(λs − λv). A model reporting half the latent heat its evaporation needs on every pack step passed with that deposition reported, and failed without sbl. An independent review pass found this; I reproduced it.

The fix:

  • sbl may not deposit more than the condensation it is a share of. The slack is 0.01 mm/day (deposition_allowance_mm_per_day), which buys at most 0.04 W m⁻². With this and the existing "no more sublimation than evaporated" bound, a split moves the required latent heat only inside the interval a model without sbl is held to. SUMMA's largest excess on the gate seeds is 0.0028 mm/day, so it trips nothing.
  • The canopy margin is now 0.5 °C, down from 2 °C. A reported canopy counts as an ice store only below, or at most 0.5 °C above, the snow threshold. SUMMA's canopy-ice sublimation with no snow on the ground falls at or below 0.07 °C on the gate seeds, and its flux_identity results are identical at 2.0 and 0.5 on all 11 runs.
  • The detail now reports deposition steps separately from sublimation steps, and other_ice_stores must be a list.
  • Four new tests: the solved-deposition case, the margin at +0.3 °C (frozen) and +1.0 °C (not), and a canopy that empties during the step. Setting the margin to 0 or 2, dropping the start-of-step canopy, or disabling the bound each turns exactly one test red.

SUMMA's two energy rows are re-run for the new detail text; no verdict moves, and it stays at 10/25. Its README now says plainly that frost steps fail: 2.42–3.92× on latent-heat, on the liquid evaporating beside the frost, not on the frost itself.

Checks on 4388ee2: pytest 1286 passed; ht validate, scripts/site_standings.py --check and the full ht gate pass.

Questions for you:

  • the 0.5 °C margin;
  • the 0.01 mm/day deposition allowance;
  • whether the "no more sublimation than evaporated" bound should get a matching allowance. It has none today, and no model trips it.

@cehw

cehw commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Thanks, Zhi — the solved-deposition regression closes a real loophole, and the smaller canopy margin keeps the ice check better targeted. I re-ran the 102 related tests at 4388ee2; all pass.

One scope clarification would help. Both bounds are useful against fabricated splits, but they do not follow from the signed energy identity. In a constructed criterion test at −2 °C, liquid condensation of 0.2 mm/day plus sublimation of 0.3 gives net evaporation of 0.1 and the correct latent flux of 4.042 W/m². The upper bound still returns FAIL, with zero energy-mismatch steps; the reverse pairing hits the new lower bound. These are output-vector tests, not native SUMMA runs.

I'd keep the guards, but describe their limits in the README and diagnostics as an additional benchmark assumption on opposing phase exchanges. Exceeding them need not mean the latent-heat identity is wrong. Removing both also lets fabricated low and high latent fluxes pass.

Your gate evidence supports the 0.5 °C margin and 0.01 mm/day allowance for these cases; I see no evidence to demand a matching upper allowance. Making this scope explicit would preserve the useful protection and make the verdict easier to interpret.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand snm coverage for packaged snow models

3 participants