Skip to content

Gridded routing docs, skills, and two Q' reader fixes - #195

Merged
taddyb merged 5 commits into
DeepGroundwater:masterfrom
taddyb:feat/ddr-skills
Sep 9, 2026
Merged

taddyb merged 5 commits into
DeepGroundwater:masterfrom
taddyb:feat/ddr-skills

Conversation

@taddyb

@taddyb taddyb commented Sep 9, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #194: the documentation that work needed, plus two bug fixes it exposed.

#194 landed the gridded routing path but left a gap — every acquisition instruction lived in wiki/, which is gitignored, so none of it reached a user. This PR closes that and fixes two defects found while writing it up.

Documentation

docs/gridded_methods.md — a methods section written for a collaborator who knows hydrology but has not seen the code. Nine sections take a reader from the DDM30 network to the limitations, with six figures from the shipped runs. It covers the three undocumented deviations in the ISIMIP product, the Courant-band derivation behind sub-reach sizing, an attribute table with the reason each source deviates from Song et al. (2025) where it does, why moving lateral inflow onto cells is mass-conserving aggregation rather than interpolation, and why the negative-coefficient regime cannot bias the water balance.

docs/gridded_data.md gains a full gauge-selection section, and the README gains a gridded section with the same in brief.

Two skills, .claude/skills/ddr-training and .claude/skills/ddr-gridded, covering the standard CLI path and the gridded path. .claude/ was ignored wholesale, so the skills library could not be committed as the project instructions require; narrowed to .claude/* with an exception for skills/.

Dataset credit — the README now names Song, Bindas et al. (2025) as the source of both the lateral inflow and the attribute set, with the Zenodo archive alongside the paper DOI.

How the gauges are selected

Documented in both the README and the data guide, because grid resolution decides the answer rather than preference. A 0.5° cell is about 2,350 km², so a basin smaller than one routing element cannot be represented.

Step Gauges
In gages_3000.csv 3,211
Drainage area > 2,000 km² 890
Snap within area tolerance 667
Observation coverage > 90% 620

Snapping searches the 3×3 neighbourhood and keeps the cell minimising |log(cell_upstream_area / gauge_drainage_area)|, with DA_TOLERANCE = (0.7, 1.4) rejecting the match if even the closest is too far off. Those bounds are asymmetric in linear terms but symmetric in log terms, which is the right symmetry for a multiplicative quantity. Nearest-centre snapping was tried first and rejected: it misplaced about half of the large gauges, by up to a factor of 201 in drainage area, because a gauge near a cell edge is often closest to a cell its river never enters.

Bug fixes

Both are instances of one failure mode: zarr answers an out-of-range read with fill values rather than raising, so a mismatched reader produces an all-NaN lateral inflow and a silently zero baseline instead of an error.

The Q′ store was written transposed. build_gridded_qprime.py emitted Qr(time, divide_id), the transpose of the contract the dHBV2 UH source follows. Regenerated the CONUS store and the shipped bundle in contract order. Values are unchanged: volume on a sample day matches the source exactly, the Juniata four-cell mean is 163.0 m³/s as before, and the example still trains to NSE 0.795 against the summed baseline's 0.594.

The benchmark read Qr positionally. _aggregate_qprime used shape[1] as the time length, correct only because the source store happens to be in contract order. Reading now goes through qprime_matrix, which selects and transposes by dimension name and refuses a variable whose dimensions match neither layout. Two tests cover it. Also removed a _t0 parameter threaded into _route_and_score that was never read.

Verified behaviour-preserving by re-running the CONUS benchmark end to end: median NSE 0.5872 against the baseline's 0.4375, beating it at 84.7% of 321 gauges, negative solves 0.0488% — identical to the numbers in #194.

Not changed, worth a decision

  • scripts/summed_q_prime.py reads Qr positionally in two places. It is the production MERIT path rather than gridded work, and it is correct for contract-order stores, but it would hit the same trap on a transposed one.
  • The gridded benchmark still builds forcing by flowline-midpoint assignment while training uses the area-weighted store, so the two measure with different inflow. About 40% of catchments straddle a cell edge.
  • The methods doc's limitations section records the two open modelling problems from Gridded (ISIMIP DDM30) routing: network, attributes, forcing, and training #194: roughness initialising at the midpoint of its bounds and pressing the ceiling after 45 epochs, and downstream hydraulic-geometry exponents that come out backwards. Neither is addressed here.

811 tests pass, 2 skipped.

taddyb and others added 5 commits September 8, 2026 22:45
ddr-training covers the standard CLI and Hydra path: picking a config,
dotted overrides for smoke tests, what the fields mean, how to read
neg_solve and parameter spread, the common failure modes, and the rule
that a run has proved nothing until it beats the summed-Q' baseline.

ddr-gridded covers the DDM30 path: the self-contained example, CONUS
training, building the stores, and the facts that are easy to get wrong -
cell and node id schemes, the 2,350 km2 resolution floor on basins,
attributes from native rasters only, forcing aggregated rather than
interpolated, why negative Muskingum coefficients are expected here, and
how sub-reach sizing targets Courant 1. It also records the two unresolved
findings so learned parameters are not presented as calibrated.

.claude/ was ignored wholesale, so skills could not be committed as the
project instructions require. Narrowed to .claude/* with an exception for
the skills library; settings.local.json and worktrees stay ignored.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YZBLnrygRbcnyQvu2KL4vz
docs/gridded_methods.md describes how gridded routing works and where every
input comes from, written for a collaborator who knows hydrology but has not
seen the code. Covers the DDM30 network and its three undocumented
deviations, sub-reach discretization with the Courant-band derivation, the
attribute sources and the native-raster aggregation rule, mass-conserving
transfer of lateral inflow, the negative-coefficient regime and why it does
not bias the water balance, parameter learning, evaluation against the
summed-inflow control, the verification suite, and the three limitations
that bear on interpreting learned parameters.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YZBLnrygRbcnyQvu2KL4vz
Six figures move from the gitignored run directories into docs/images so
the methods section shows its results rather than asserting them: the
learned parameter maps, the Juniata hydrograph against the summed-inflow
control, continental metrics, parameter convergence, the channel-geometry
check, and parameter relations. Each carries a caption stating what it
shows. The convergence and geometry figures sit in the limitations section,
where they are evidence for the two unresolved problems.

*.png was ignored wholesale, so documentation figures could not be
committed; added an exception for docs/images.

README gains a gridded routing section and credits Song, Bindas et al.
(2025): the lateral inflow and the attribute set both come from that
dataset, now cited with its Zenodo archive alongside the existing paper
citation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YZBLnrygRbcnyQvu2KL4vz
Grid resolution decides which gauges are usable, so selection is a filter
chain rather than a preference: 3,211 in gages_3000, 890 above 2,000 km2,
667 snapping within drainage-area tolerance, 620 with sufficient
observation coverage. Both the README and the data guide now show the
funnel with the control for each step.

Records why nearest-centre snapping was rejected, since it is the obvious
first thing to try: it misplaced about half of the large gauges, by up to
a factor of 201 in drainage area, because a gauge near a cell edge is often
closest to a cell its river never enters. Matching on accumulated area
tests the hydrologic connection instead of assuming proximity implies it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YZBLnrygRbcnyQvu2KL4vz
DA_TOLERANCE = (0.7, 1.4) bounds the ratio of a cell's accumulated area to
the gauge's reported drainage area. Records why the bounds are asymmetric
in linear terms but symmetric in log terms, since drainage area is
multiplicative and the matching minimises absolute log error; that it
rejects rather than constrains the search; and that it binds near the
resolution floor rather than for large basins.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YZBLnrygRbcnyQvu2KL4vz
@taddyb
taddyb merged commit 4d5fb39 into DeepGroundwater:master Sep 9, 2026
4 checks passed
@taddyb taddyb changed the title Add ddr-training and ddr-gridded skills Gridded routing docs, skills, and two Q' reader fixes Sep 10, 2026
taddyb added a commit that referenced this pull request Sep 11, 2026
scripts/summed_q_prime.py indexed Qr positionally in two places, taking axis
0 as divides and axis 1 as time. That is correct only for a store written in
contract order, and the failure is silent rather than loud: zarr answers an
out-of-range read with fill values instead of raising, so a transposed store
yields an all-NaN lateral inflow and a baseline of zero.

Both reads now go through ddr.io.readers.qr_as_divide_time, which selects by
dimension name and refuses a variable whose dimensions match neither layout.
The helper lives in the reader module rather than the script because the
script imports cupy at module scope and so cannot be imported without a GPU,
which is why the existing tests reimplement its logic inline.

Same class of defect as the gridded fixes in #195, on the MERIT path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YZBLnrygRbcnyQvu2KL4vz
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