Stabilize modeled-climatology change factors and fix CONUS CSV/JSON divergence - #739
Conversation
The CSV branch returned before calculate_and_apply_gcm_diffs_to_maurer_climatology ran, so format=csv served raw original-GCM values while JSON served the Maurer-adjusted values -- under a CSV header claiming the values were adjusted. Move the adjustment ahead of the CSV export so both formats serve the same data. Companion to the arctic_hydrology fix on the json_csv_discrepancy branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Blaskey and Maurer adjustments scaled each statistic by gcm_projected / gcm_historical with only a one-sided floor on the denominator (0 -> 0.0001). When the historical minimum is near zero -- routine for frozen arctic streams in winter -- the ratio explodes: at stream 81009008, doy 119, C2LE2's doy_min came out as 602,159.949 cfs (1.223 * 492.363/0.001), far above its own doy_max. Replace the raw quotient with a stabilized ratio shared in postprocessing: ratio = (projected + eps) / (historical + eps), clamped to [0.1, 10] where eps is 1% of the model's historical mean flow, so the offset scales from headwater creeks to large rivers. As both values approach zero the factor now approaches 1 (no change) instead of diverging. Unit tests reproduce the production blowup case and cover the epsilon scaling, clamping, and structure handling of both adjustment functions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Because each statistic is scaled by its own change factor, adjusted values could violate basic ordering even at ordinary magnitudes: for CONUS stream 50563 (dynamic/CCSM4/rcp26, doy 214) the adjusted doy_min (353.042) exceeded the adjusted doy_max (343.703), and 409 of 115,656 entries for that stream were internally inconsistent. Order statistics have noisy ratios -- a single flood day in the GCM historical era deflates the max's factor without touching the min's. After adjustment, clamp doy_min down to doy_mean and doy_max up to doy_mean so every published triplet satisfies min <= mean <= max. Tests cover the production doy-214 crossing case and a synthetic Blaskey crossing case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses correctness issues in modeled hydrology climatology outputs by (1) ensuring CONUS CSV exports match JSON (both using the Maurer-adjusted data when requested) and (2) stabilizing the GCM change-factor math to prevent near-zero baseline blowups and enforce post-adjustment min ≤ mean ≤ max.
Changes:
- CONUS modeled climatology: apply the Maurer adjustment prior to CSV serialization so CSV and JSON return the same adjusted values for
source=gcm_diff_applied_to_maurer. - Introduce
scale_aware_epsilon()andstabilized_ratio()(with caps) to prevent ratio explosions near zero and make the behavior scale-aware across streams. - Enforce
doy_min ≤ doy_mean ≤ doy_maxafter applying per-stat ratios for both Arctic (Blaskey) and CONUS (Maurer) adjustments; add focused unit tests reproducing production failures.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/test_hydrology_adjustments.py |
Adds unit tests covering stabilized ratios, ordering enforcement, PGW skipping, and structure preservation for both adjustment paths. |
routes/conus_hydrology.py |
Stabilizes Maurer adjustment ratios, clamps stat ordering, and moves adjustment before CSV generation to eliminate CSV/JSON divergence for CONUS. |
routes/arctic_hydrology.py |
Stabilizes Blaskey adjustment ratios and clamps stat ordering to prevent extreme/invalid adjusted outputs. |
postprocessing.py |
Adds shared constants plus scale_aware_epsilon() and stabilized_ratio() helper functions for robust ratio-based adjustments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
charparr
left a comment
charparr
left a comment
There was a problem hiding this comment.
OK, I've gone through the logic in postprocessing.py and the implementation in the routes pretty carefully. I think we are nailing the desired behavior here...the stabilized ratio (projected + epsilon) / (historical + epsilon) is shrinking poorly constrained low-flow change factors toward a value of 1 (i.e. no change) but converging on the established ratio when the baseline is large relative to epsilon. I think the tests coverage is a nice addition here because we've it covers the blowups we saw before, and the min/mean/max ordering which is an important part of this PR. I did leave some comments that are mostly regarding the assumptions that we are baking into this code...and maybe these should get documented somewhere / ticketed for later consideration if they need to be conveyed to users. The implementation is strong and I have zero issues merging this as is.
| RATIO_EPSILON_FLOOR = 0.0001 | ||
| RATIO_CAP = 10.0 |
There was a problem hiding this comment.
One immediate question I have is how frequently are the floor / cap values triggered? If these values are returned frequently then they might need adjustment. It seems like they are intended to catch outliers which I think is the right approach but it could be worth an empirical test before all is said and done. I do see that Claude called these out already as "judgement calls" that Claude is "happy to tune".
| """ | ||
| if not values: | ||
| return RATIO_EPSILON_FLOOR | ||
| epsilon = RATIO_EPSILON_FRACTION * (sum(values) / len(values)) |
There was a problem hiding this comment.
One choice I see embedded here is that the arithmetic mean is the value used to scale the epsilon. This makes sense to me because it is easy to explain and to conceptualize: big historical flows get bigger epsilons, and smaller historical flows get smaller epsilons.
But, are there any streamflow regimes for which this might have unanticipated consequences? Imagine a stream where the flow is: [small, small, small, BIG BIG GUSH O WATER, small, small, ...] : the mean may not scale the epsilon value in a desirable way because the big flow might get only get scaled a tiny bit.
Down the road it may be worthwhile exploring the use of the median the scale factor, or some other statistic.
And/or just adding some more text along the lines of
"Hey for a stream that is highly variable or seasonal, a spike in flow is going to raise the mean, which will raise the epsilon for the entire year's worth of flows.
| doy_stats[stat] = blaskey_adjusted | ||
| # each stat is scaled by its own ratio, so the adjusted values | ||
| # can cross; clamp to preserve min <= mean <= max | ||
| if all(k in doy_stats for k in ("doy_min", "doy_mean", "doy_max")): |
There was a problem hiding this comment.
Good comment on the "why" -- feels like we'll come back to this
| # each stat is scaled by its own ratio, so the adjusted | ||
| # values can cross; clamp to preserve min <= mean <= max | ||
| if all( | ||
| k in doy_stats for k in ("doy_min", "doy_mean", "doy_max") | ||
| ): | ||
| doy_stats["doy_min"] = min( | ||
| doy_stats["doy_min"], doy_stats["doy_mean"] | ||
| ) | ||
| doy_stats["doy_max"] = max( | ||
| doy_stats["doy_max"], doy_stats["doy_mean"] | ||
| ) |
There was a problem hiding this comment.
I think this is identical to R543 to R551 in routes/arctic_hydrology.py
And thematically it feels like postprocessing. Consider consolidating and moving to the post-processing module. If nothing else the route is already 1000+ LOC so it could be nice. However, this suggestion is not functional so just take it under consideration.
Background
Investigating
arctic_hydrology/modeled_climatology/81009008revealed that the JSON output reported adoy_minof 602,159.949 cfs for C2LE2 at doy 119 (2034–2065) — ~70× above its owndoy_max— while the CSV export of the same endpoint showed entirely different numbers. Three distinct defects were isolated, all verified numerically against production:json_csv_discrepancybranch fixes this for the two Arctic routes; this PR intentionally leaves those hunks to that branch and fixes only the CONUS route (no conflicts — different files/regions).gcm_projected / gcm_historicalexplodes when the historical minimum is near zero (0.001 cfs winter flows on frozen streams). The one-sided0 → 0.0001floor makes this worse, not better. The 602,159.949 value is exactly1.223 × (492.363 / 0.001).min ≤ mean ≤ maxeven at ordinary magnitudes. For CONUS stream 50563, 409 of 115,656 adjusted entries were inconsistent (raw data: 0), e.g. doy 214 CCSM4/rcp26: adjusted min 353.0 > adjusted max 343.7.Changes (one commit per fix)
Commit 1 — CONUS: apply Maurer adjustment before CSV export. Mirrors the Arctic fix on
json_csv_discrepancy; both formats now serve the same adjusted data, matching the CSV header's description.Commit 2 — Stabilized change factors (shared helper in
postprocessing.py, used by both Blaskey and Maurer functions):[0.1, 10]cap is a backstop against noisy order statistics; ordinary ratios (e.g. mean flows on healthy baselines) are essentially unchanged (< 2% shift at 81009008).doy_mingoes from 602,159.949 → ≈ 12.2 cfs.Commit 3 — Enforce
min ≤ mean ≤ maxafter adjustment: clampdoy_mindown todoy_meananddoy_maxup todoy_mean.Testing
tests/test_hydrology_adjustments.py(14 tests): reproduces the production doy-119 blowup and the doy-214 ordering violation with real production values; covers epsilon scaling, clamping, PGW-model skipping, and structure preservation for both adjustment functions.test_landslidefailures pre-exist onmain(require a live database).Notes for review
historicalscenario as an exact copy of Maurer post-adjustment (redundant payload); the water-temperature additive adjustment can also produce min/mean/max crossings; a methodologically stronger long-term fix is computing change factors on 31-day windows (or applying deltas to the daily series upstream in arctic_rivers) — see the investigation writeup for details.🤖 Generated with Claude Code