Skip to content

AORC2F/LSTM Q' routing campaign: 4-arm comparison + GPU eval OOM fix#27

Merged
taddyb merged 8 commits into
masterfrom
aorc2f-lstm-qprime-campaign
Jul 22, 2026
Merged

AORC2F/LSTM Q' routing campaign: 4-arm comparison + GPU eval OOM fix#27
taddyb merged 8 commits into
masterfrom
aorc2f-lstm-qprime-campaign

Conversation

@taddyb

@taddyb taddyb commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

  • Ran 4 CONUS train-and-test arms with the frozen capacity-boosted disagg head against alternative Q' forecast stores (AORC2F distributed, AORC2F lumped, daily-lstm, hourly-lstm), comparing against the standing 0.7152 NSE / 0.7106 KGE benchmark and each arm's own summed-Q' baseline. None beat the benchmark; daily-lstm came closest (0.5674/0.6169). Full results in docs/2026-07-16-aorc2f-wave1-findings.md and docs/2026-07-16-wave2-cross-wave-findings.md.
  • Found and fixed a real bug twice: a cubecl-cuda background-thread OOM panic during GPU eval that never propagated to the caller, silently corrupting predictions instead of failing. Fixed in two stages (value heuristic, then a panic-hook detector after the heuristic proved insufficient on a second occurrence) — see the two fix(eval) commits. 6 new unit tests.
  • Fixed a pre-existing, unrelated doctest compilation failure found while running the full test suite.
  • Confirmed via an input-perturbation sweep that the frozen disagg head is byte-identical and genuinely precip-responsive across all three arms that load it.
  • Documented the OOM incident + fix in the ddrs-run-and-operate skill (§14).

Test plan

  • cargo build --release — clean
  • cargo test --release — 81/81 test binaries pass, 0 failures (including doctests after the fix)
  • cargo run --release --example compare_ddr_sandbox — ABSOLUTE MATCH (DDR-parity regression gate)
  • cargo test --lib training::eval::tests — 6/6 pass (corruption-detection unit tests)
  • All 4 experiment configs run to completion end-to-end (train-and-test), metrics cross-checked against manifest.json / predictions.zarr

🤖 Generated with Claude Code

taddyb added 8 commits July 15, 2026 22:19
…y templates

New train-and-test configs for the AORC2F dHBV Q' forecast stores
(distributed+UH-routing and lumped variants) and the NH-LSTM daily/hourly
stores, all using the 2026-07-12 capacity-boosted frozen disagg head
(hourly-lstm omits it, since disagg + an hourly-native source is a config
error). Also corrects docs/nh-qprime-store-contract.md's stale divide count
for daily_dhbv2_merit_unit_catchments.ic (288,421 -> 197,088; the store was
rebuilt 2026-07-10 after the doc was written) and adds the untracked
daily_dhbv_aorc2f_merit_unit_catchments.ic store to the table, both verified
via `ddrs import --dry-run`.
…ad predictions

Incident: a `--backend cuda` train-and-test run's Phase 2 (testing) hit a
CUDA OOM inside a cubecl-cuda background worker thread ("can't allocate
buffer of size: ..."). The panic never propagated to the caller -- the main
thread kept looping through all 366 eval chunks, writing whatever
half-written GPU buffer was left, instead of failing.

Adds `DataError::CorruptedEvalChunk` and a `corrupted_chunk_reason` check
(all-zero or non-finite chunk values) run after every eval chunk in
`evaluate()`, which now takes a `checkpoint_path` for error context. Threads
the new parameter through all four call sites (`cli/run.rs` Phase 2,
`bin/eval.rs` both EvalParams arms, `bin/train_and_test.rs`,
`tests/training_verification.rs`). Also adds a best-effort
`system::free_gpu_gb()` warn-level log before Phase 2 begins (not gated --
cubecl's retained memory pool makes a point-in-time threshold check
unreliable).

4 unit tests cover corrupted_chunk_reason (healthy/all-zero/nan/inf).
A second, independent occurrence of the OOM incident from the previous
commit showed the value-based corrupted_chunk_reason check is NOT
sufficient on its own: the corrupted chunk happened to contain
plausible-looking finite, non-zero values (stale GPU memory left over from
a prior op) that slipped past the all-zero/non-finite heuristic while the
cubecl-cuda worker thread panicked dozens of times underneath.

Adds a process-global WORKER_PANICKED flag set by a custom panic hook
(ensure_panic_hook_installed, wraps rather than replaces the default hook so
panic messages still print) -- a reliable detector, since it fires on ANY
background-thread panic regardless of what ends up in the output buffer.
Checked after every chunk and once more after the post-loop tensor readback
(tau-trim/downsample + zeta-mean readbacks also run on-device and were
previously unchecked). 2 new unit tests (6 total in this module) confirm
the hook fires across threads and the flag clears correctly between calls.
Three doc comments embedded arrow-diagram/formula pseudocode as an indented
block (4-space indent after `///`), which rustdoc's default doctest harness
tries to compile as runnable Rust regardless of whether it's valid syntax.
Pre-existing, unrelated to this session's other changes -- surfaced by
running the full `cargo test --release` suite (deferred earlier in the
session to avoid CPU contention with in-flight training runs). Wraps each
in an explicit ```text fence so rustdoc treats it as documentation, not a
test. `cargo test --release --doc` now passes (was 4 failing to compile).
…nd-operate

Adds section 14 covering the cubecl-cuda silent-corruption failure mode
(symptom, root cause, the fix from the two previous commits, and how to
tell if your installed binary predates it), a new Common Failure Checklist
row, a cross-reference from the Performance Baseline section noting
alternative Q' stores score below the documented CONUS benchmark, and
updated frontmatter/Provenance sections pointing at this session's findings
docs and verification greps.
…gg sensitivity

Four CONUS train-and-test arms against alternative Q' forecast stores
(AORC2F distributed, AORC2F lumped, daily-lstm, hourly-lstm), all using the
frozen capacity-boosted disagg head where applicable, compared against each
arm's own summed-Q' baseline and the standing 0.7152/0.7106 CONUS benchmark:

  arm                  trained NSE/KGE   own baseline NSE/KGE
  AORC2F distributed   0.3437 / 0.3256   0.233 / 0.327
  AORC2F lumped        0.5259 / 0.5175  -0.105 / 0.393
  daily-lstm           0.5674 / 0.6169   0.437 / 0.616
  hourly-lstm          0.5543 / 0.4852   0.532 / 0.547

None of the four stores beat the standing benchmark under this routing
configuration; daily-lstm comes closest. Also includes an input-perturbation
check confirming the frozen disagg head is byte-identical and genuinely
precip-responsive across all three arms that load it (expected, since all
three warm-start from the same pretrained checkpoint with freeze: true).

scripts/wave_comparison_plots.py generates the NSE/KGE box plot and CDFs
across all 4 trained arms + their baselines (output/2026-07-16-wave-comparison/).
…sis + missing controls

A 2026-07-21 adversarial review (fable subagent, prompted by the user
flagging the AORC2F-distributed arm's result as suspicious) found the wave
1 doc's dismissal of the GPU-train/CPU-eval backend mismatch cited an
inapplicable test (tests/sparse_cusparse_v5.rs bounds one forward+backward
solve, not 5 epochs of Adam-driven divergence). Replaces it with the actual
decisive evidence: mb=0 training loss, logged before any weight update from
the identical seed-42 init, already orders all four arms exactly as the
final results do -- the ranking is data-borne, not a training-backend
artifact.

Adds the review's leading untested hypothesis: the "distributed + UH
routing" AORC2F store may already have unit-hydrograph routing applied
upstream, so ddrs's Muskingum-Cunge routing on top would double-route it --
consistent with both its modest-but-not-negative raw baseline and its
below-the-rest routing gain. Also records the review's list of missing
controls (no replicate seeds, no backend-consistent control run, no
per-gauge/spatial diagnostics, benchmark reference predates mid-campaign
eval-code changes).
Read Song, Bindas, Shen et al. (2025, WRR) -- the paper this model family
descends from -- which clarifies that their own MC-routing arm is fed q_m
(pre-aggregation, pre-UH-routing per-unit runoff), not q_b (the UH-routed,
one-value-per-gage-basin streamflow). Directly tested the naive version of
the double-routing hypothesis against daily_dhbv_aorc2f_merit_unit_catchments.ic:
pulled Qr for 20 COMIDs spanning a 578-reach gauge subgraph (USGS 01184000)
via icechunk/zarr and checked for bit-identical pairs, since q_b is
architecturally incapable of native per-unit resolution. Result: 0/190
identical pairs in either the distributed or lumped store -- refutes the
strong form (replicated basin-outlet flow). A secondary smoothness check
(lag-1 autocorrelation, day-to-day volatility) found distributed somewhat
smoother than lumped, a weak and inconclusive signal for a milder partial-
routing variant. Recommends checking the actual export script's semantics
directly rather than continuing to infer from store contents alone.
@taddyb
taddyb merged commit 81f9b80 into master Jul 22, 2026
2 checks passed
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.

1 participant