Verification metrics overhaul: timing resolution, unified scoring, and honest diagnostics - #2
Open
rhuber6 wants to merge 128 commits into
Open
Verification metrics overhaul: timing resolution, unified scoring, and honest diagnostics#2rhuber6 wants to merge 128 commits into
rhuber6 wants to merge 128 commits into
Conversation
Correctness fixes: - fetchForecasts returned a Map in *completion* order, not date order, because its 4 concurrent workers set entries as each resolved. reorganizeByLead inherited that order, so every lead bucket came out unsorted while five metrics assumed sorted input via time[0]/time[last] — corrupting the overlap window and, in thresholdCrossing, the meaning of "first ascending crossing". Re-key in date order and enforce chronological order per bucket. - Plotly counts data-coordinate shapes in autorange, so return-period bands drawn as layout shapes pinned the y-axis at a hidden trace's peak. Bands are now filled traces (rpBandTraces), which drop out of autorange when hidden; the top band is stretched to the visible data by syncTopBands in Plot.tsx. Temporal resolution: - resampleHourly interpolated coarse uploads *up* to hourly, inventing 23 points per real daily observation and inflating every sample count ~24x. Deleted. Uploads keep their native cadence; comparison now happens at the coarser of observation/forecast cadence (cadence.ts, grid.ts), aggregating the finer side down. Bin mean for error metrics, bin max for threshold metrics, since a bin mean can hide an exceedance. Metrics: - Remove the deterministic-limit plot: raw counts with a varying denominator, lead 0 structurally incomparable, and a censored limit reported as exact. - Add CRPSS (skill score vs seasonal climatology), NSE, per-run peak timing, and NSE/KGE' skill-summary bars by lead day and by forecast run. - Guard correlation-shaped scores below 10 pairs and show the reason on the plot rather than emitting a bare NaN. - Contingency matrix now exposes per-timestep detail, so the observed-vs- forecast graph beside it cannot disagree with the table. Docs: interpretation notes under all 14 plots, plus an Overview section stating the resolution contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds a line-by-line port of geoglows.bias.correct_forecast (v2.2.0) plus a Vitest harness that proves bit-for-bit parity against the real Python package. Closes plans/02-architecture.md open question 5 (fixture-based parity tests). Why a port and not a Python serverless function: all three inputs already exist client-side (app.forecasts, app.retro, app.historicalData), while `pip install geoglows` is 814 MB against Vercel's 500 MB Python limit — so a backend would require vendoring bias.py, which is a fork with the same audit burden in a second language, and would end the app's static-bundle property. Fixtures are generated by scripts/gen-bias-fixtures.py calling the installed package directly, never a transcription — validating against a hand-copied source would pass even if the copy were wrong. The generator asserts its recomputed intermediates equal the returned interp1d's own .x/.y before writing them, and stores records deduplicated and month-trimmed (12 MB -> 874 KB). Reference behaviours reproduced deliberately, each pinned by a fixture: - scipy interp1d uses its slope form, not np.interp, when fill_value='extrapolate'. The two disagree on tied x-values; the naive form diverges ~31% at p=1.0 where the upper flow range lands. - pandas DataFrame.update does not overwrite where the incoming value is NaN, so a NaN mapping retains the RAW forecast value. Fixture `nan-mapping-keeps-raw` shows raw [1, 2, 85, 95] -> [1, 2, 76.125, 91.35]: the first two are uncorrected but look entirely plausible. - counts are divided element-wise before sequential accumulation. That single ULP in cdf[last] decides whether the inverse mapping returns a finite ceiling or +Infinity, so cdf[last] is asserted in hex. - arange computes start + i*step, never accumulating. - bins deliberately overshoot the data, which is what creates the flat CDF tail that makes the inverse undefined. The two dead `bins[0] >= 0` branches are omitted; minVal is computed but unused, as upstream. Verified by mutation testing — all five plausible wrong implementations are caught: single-expression interpolation (13 failures), runningCount/n cumsum (32), accumulated arange (25), searchsorted side=right (7), and last bin right-open (2). The last needed a direct np.histogram fixture, since buildMonthlyCdf's bins can never reach the final edge. Harness: tests live in a root tests/ tree with their own tsconfig.test.json project, keeping fixture JSON out of the app program — tsconfig.app.json has include:["src"] with no exclude and no resolveJsonModule, so a test under src/ would break `npm run build`. 93 tests passing. No app behaviour changes yet; wiring is the next step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moves ForecastRun into src/lib/types.ts and widens reorganizeByLead and skillByRun to accept it. A locally-derived corrected run cannot supply ForecastResult's `stats` field (only src/plots/forecasts.ts reads it, and that plot stays raw-only), but ForecastResult remains structurally assignable, so no call sites changed. Adds correctForecasts(): builds one monthly mapping per calendar month, shared across every run in that month, and applies the exclusion policy. Exclusion is at RUN granularity, not timestep. +Infinity arises only for forecast values above the simulated monthly maximum — the peaks — so dropping individual timesteps would systematically delete the highest flows and flatter every corrected score. Whole-run exclusion is also the granularity skillByRun and peakByRun need, since they read the Map directly. nanKeptRaw and negativeClipped are reported but not excluded. Writing the driver surfaced how common the NaN path is: any forecast value below the simulated monthly minimum lands in the leading flat-zero region of the CDF, maps to p = 0, and the inverse is undefined there — so the reference retains the raw value. That is routine for low-flow timesteps, so excluding those runs would discard most of a dry month for nothing. It does mean those cells are uncorrected, which is why the count is surfaced rather than swallowed. Coarser-than-daily observations are refused outright: aggregateSeries only downsamples, so such a record cannot be brought onto the CDF grid and would otherwise be silently mis-binned. 102 tests passing. Still no app behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mary Wires the ported correction into the three metric families that compare raw discharge. Each gets a "Forecasts: raw / bias-corrected" select, defaulting to raw, following the existing labelled-<select> idiom (there are no checkbox or toggle components anywhere in src/). Categorical and Timing are deliberately left raw-only: their dual-threshold classification already absorbs magnitude bias, so correcting the forecast as well would apply the adjustment twice. The seam is upstream of lead bucketing. Corrected forecasts are built as a parallel Map and pushed through the same reorganizeByLead -> aggregateBucket chain, because quantile mapping is nonlinear -- correcting a bin mean is not the mean of corrected values. The corrected side reuses the SAME grid rather than deriving its own: excluding runs removes timestamps, which could shift bucketCadence's median and select a different grid, and two variants on different grids are not comparable. Only 'mean' is aggregated for the corrected side, since the 'max' families stay raw. app.leadBuckets keeps meaning *raw* -- ForecastTab reads it -- so corrected state is local to MetricsTab rather than in AppContext. CRPSS climatology change: it now always comes from the observed record, for both variants, and is REQUIRED. The reference is scored against observations, so a baseline built from model output is biased wherever the model is, which makes it artificially easy to beat; and one shared reference is the only way CRPSS_corrected - CRPSS_raw reflects a change in the forecast rather than a change in the denominator. Without the historical upload CRPSS is omitted with an explanation -- CRPS itself needs no climatology and still renders. Raw CRPSS values will therefore differ from previous builds; that is the intended effect, and the plot subtitle now names its climatology source. Extractions to keep both variants provably identical in code: countPairs, griddedFor and accuracyDistributions are now module-level pure functions, so raw and corrected are computed by the same path and any divergence would be a real difference in the forecasts rather than a difference in the code. A correction banner reports what actually happened: which months were mapped and their in-season sample sizes, which runs were excluded and why, how many member-timesteps kept their raw value because the mapping was undefined, and how many were clipped. Excluded runs also appear as labelled n/a bars on the skill-by-run chart, so a run disappearing is visible rather than silent. Docs: new Overview section covering the method, why only three families are corrected, and the six limits that matter -- infinity on extreme forecasts, raw values retained below the simulated monthly minimum, daily distributions compressing sub-daily peaks, short records lowering the ceiling, in-sample correction, and dry-month collapse. Setup's historical uploader label now names all three things it gates. 102 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RFS (River Forecast System) is the hydrological model; GEOGLOWS is the programme
and data service it is published through. UI copy I added was using GEOGLOWS as
the model's name, which conflates the two.
The Overview introduction now defines both terms once, and model behaviour is
attributed to RFS throughout ("RFS is a global model", "the RFS retrospective",
"RFS issues a new ensemble forecast every day"). References to GEOGLOWS as the
programme are left alone, as is the app title and "GEOGLOWS RFS forecasts",
which were already correct.
Code identifiers are untouched: the Python package genuinely is named geoglows
(geoglows.bias.correct_forecast) and the npm client riverforecastsystem, so both
names legitimately appear in the codebase for different things — the Overview
note about the port now says so explicitly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
New "Bias correction" block on the Metrics tab holding four views. No metric here — these exist to show what the correction does, since until now its behaviour was only described in prose. 1. Transfer curve. Simulated flow in, corrected flow out, against a 1:1 reference, with the actual forecast values as a rug so you can see which part of the curve your event uses. Two lines deliberately: the raw quantile map (broken where the inverse is undefined) and what the app actually applies (falling back to the raw value in those gaps). This makes the method's limits visible instead of documented — the inflation factor is the departure from 1:1, the no-op region is where Applied rejoins it, and the exclusion threshold is where the map runs off the top. The subtitle names both thresholds in m3/s. 2. The two monthly CDFs, drawn as steps. Horizontal distance between them is the bias being removed; the flat treads are exactly where the inverse fails. 3. Correction shift per lead day: (corrected - raw) across members. Matched on timestamp, not index, because excluded runs remove their timesteps entirely and positional differencing would compare unrelated instants. 4. One run before and after, against the observations. The plainest test of whether the correction helped, and it counts the timesteps it left untouched. The plots draw from correction.mappings, now exposed on BiasCorrection, rather than rebuilding the mapping — a rebuilt curve could drift from the one actually applied, which is the one failure this block exists to rule out. Note stated on plot 3 and in its interpretation note: the transfer curve is the same at every lead, because the simulated distribution comes from the retrospective, which has no lead dimension. Any lead-dependence in the shift is those leads occupying a different part of one fixed curve, not the correction treating them differently. That is the method's main structural limit — forecast error grows with lead and the correction cannot know it. 102 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects visible in one screenshot of the skill-summary bars. 1. Pair counts came from whichever member happened to be last. skillByLead and skillByRun both did `pairs = res.n` inside the member loop, so the reported count -- and therefore the minPairs exclusion -- was decided by the final member alone. Members do NOT share coverage: the fetched ensemble union-joins the 3-hourly members with the hourly high-resolution member onto one index and pads the gaps, which measured 8200 NaN of 14560 on river 210265545. So a single sparse member excluded whole leads that 50 other members covered fully, producing the nonsensical pattern of leads 3 and 4 reporting "0 pairs" while leads 5 and 6 reported 8 and 7. `pairs` is now countAlignedPairs (new, in alignment.ts): the timestamp overlap, independent of any member. Members are additionally required to clear minPairs individually before their score enters the median -- previously a member with four aligned points contributed a wild-but-finite KGE' alongside members with thirty. The two failure modes now report distinctly: "only N overlapping timesteps" versus "no member had N usable pairs (best M)". 2. One catastrophic score made the whole panel unreadable. Both scores are unbounded below, and an NSE of -1250 -- entirely reachable on a 6x-biased reach -- stretched the axis until every other bar was a hairline. Below about -1 the exact value carries no further meaning; it is all "far worse than predicting the mean". Bars now clip at a -1 floor and are labelled with their true value (◄ -1250), hover reports the unclamped number, and the subtitle counts how many rows were clipped. 5 new tests cover the counting fix, including the exact shape that caused it: a bucket where every member has full coverage except the last. 107 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verified the port against the Python package on real data for river 210265545 and station 8112: byte-identical, max abs difference 0.000e+00 across all 6120 cells, same range and same non-finite count. The driver plus bucketing path matches the standalone port cell for cell too. So the implementation is right. What is wrong is the METHOD on this reach, and it was hiding in plain sight: the inflation factor came out as exactly 11.87x at every single lead day. A quantile mapping should not be a constant multiplier. Cause: Sturges' rule fixes the bin COUNT (13 here) and the width comes from the range, so a right-skewed record -- normal for streamflow -- piles almost everything into the first bin. For August at this reach, 81% of simulated and 68% of observed values fall in one bin, and 99.4% of forecast values land inside the simulated first bin. Every value therefore sits on the first straight segment of one CDF and maps onto the first straight segment of the other, and two straight lines composed is a single scale factor. The mapping has no resolution where the data actually is, which is also why corrected NSE collapses: everything is scaled up uniformly, low flows included, so squared error explodes. MonthDiagnostic now carries simMaxBinShare, obsMaxBinShare and a lowResolution flag (both sides >= 50% in one bin), and the correction banner spells it out and points at the transfer curve, which shows the near-straight line directly. Also corrects two claims I had made on false premises: - The "8200 NaN of 14560" was an artifact of the PYTHON client, which returns 52 members with hourly and 3-hourly series union-joined onto one index. The app's npm client returns 51 members x 120 steps at uniform 3h with ZERO non-finite cells, verified by fetching through it. That padding does not exist in the app. - The erratic per-lead pair counts therefore were not caused by a sparse last member. The earlier fix stands on its own merits -- `pairs = res.n` inside the member loop was genuinely wrong, and per-member sample guards are right -- but the rationale I gave for it was not. 107 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Inspecting the real event file (8112_RT) explains the wild metric values far
better than bias correction did. It is hourly, spans 3.4 days in October 2024,
and its median of 12.4 m3/s sits right on October's climatological median of
10.59 -- but it contains:
2024-10-29 18:00 2232.954
2024-10-29 19:00 .. 23:00 blank (5 hours, dropped by the parser)
2024-10-30 00:00 9609.126 <- 12x the 110-year record of 800
2024-10-30 01:00 2972.240
2024-10-30 02:00 94.201
A five-hour gauge outage straight through the peak, and the first reading after
it is twelve times anything in 110 years. That single value sets
eventReturnPeriod to 100 and owns nearly all of a 3-day event's variance, so NSE
and KGE' stop measuring forecast skill and start measuring whether the forecast
produced a 9609 m3/s spike -- which nothing will, corrected or not.
Adds assessEventData(): detects runs of missing timesteps at the series' own
cadence and values above the historical maximum, and a Setup-tab notice that
names them with timestamps and multiples. Verified against the real files: it
reports all three readings above 800 m3/s (12.0x, 3.7x, 2.8x). Both problems are
invisible in a plot of a short event and silently dominate every magnitude
metric, which is why they belong at the point of upload.
Also declutters the skill-summary bars. Repeating an identical reason on every
skipped row buried the bars themselves; reasons shared by more than two rows are
now stated once in the subtitle with the row range, and those rows carry a
compact "n/a" instead. Reasons unique to one or two rows still print inline.
Note the earlier screenshot predates the pairs=res.n fix: with hourly event
observations on a 3-hourly grid there are ~27 pairs per lead, comfortably above
the threshold, so most of those rows should now score.
107 tests passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reproduced the reported pattern exactly by running the full pipeline on the real
event (8112_RT, 2024-10-28..31) with the 19 real forecast runs for that window:
lead | RAW pairs | CORRECTED pairs
1 | 27 | 12
2 | 27 | 4
3 | 27 | 0
4 | 27 | 0
5 | 27 | 8
10 | 27 | 27
Raw is 27 at every lead -- 3.38 event days x 8 three-hourly bins, exactly right.
The corrected side collapses because 6 of 19 runs were excluded, and the six are
the ones nearest the flood:
20241023: 4 infinities 20241027: 82
20241025: 24 20241028: 71
20241026: 53 20241029: 271
Exclusion fires when a forecast exceeds the simulated monthly maximum, which is
precisely what a run that predicts the event does. So exclusions concentrate on
the runs that saw the flood, and the survivors are disproportionately the runs
that missed it. At leads 3-4 the only runs covering the event window are the
excluded ones, hence exactly zero; by lead 10 the contributing runs are early
October, which never saw the flood, so all 27 pairs return.
That makes the corrected variant a selection-biased subset: it can only show
skill on the forecasts that failed. Worse than no answer, because it looks like
one.
My original justification for run-level exclusion was backwards. I argued that
timestep-level exclusion "would delete exactly the peaks" -- but dropping whole
runs deletes the peaks AND everything else in the runs that mattered most, and
does so systematically.
correctForecasts now reports selectionBias when more than 10% of runs are
excluded for infinity, and MetricsTab treats the corrected variant as
unavailable in that case, with the reason stated at the top of the correction
banner instead of quietly serving the biased subset.
This does not fix the underlying method -- month 10 also reports lowResolution,
so the transfer curve is close to a constant multiplier here. It stops the app
presenting a meaningless number as a result.
107 tests passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A parallel code audit caught that the guard added in 891de5a was inert. It set `correctedAvailable` from `selectionBias`, but that variable only fed the reason string -- the three VariantSelect controls gate on correctedAccuracy / skillLeadCorrected / correctedCrps, all of which derive from griddedCorrected, which never checked it. So the corrected option stayed enabled and still served the biased subset. The guard was decorative. Gated at the source instead: griddedCorrected returns null when selectionBias is set, so all three corrected datasets become null and every select disables from one check. skillRunCorrected reads correction.forecasts directly rather than griddedCorrected, so it carries its own gate. Adds tests/bias/leadPairGeometry.test.ts, pinning the mechanism as a closed form with no network. reorganizeByLead gives each run one contiguous 24-hour block per lead, so pairs at a lead are a plain SUM of all-or-nothing per-run blocks; excluding whole runs makes the count a step function over which init dates survived, with no reason to be monotonic. Excluding the six real dates (20241023, 25, 26, 27, 28, 29) reproduces the measured vector exactly: [2, 12, 4, 0, 0, 8, 7, 16, ...] with 27 at leads 10-14 The audit reached the same six dates analytically from the screenshot alone, by brute-forcing all 2^19 subsets: 172 reproduce it and every one excludes exactly that set. Independent agreement with the direct measurement. The tests also pin the discriminator that distinguishes this from missing data: a zero-pair lead still has a NON-EMPTY bucket, so the reason reads "only 0 overlapping timesteps" rather than "no forecast data". And they pin non-monotonicity (4 -> 0 -> 0 -> 8), which is the signature of run exclusion rather than of absent observations. Writing the test also corrected a detail I had wrong earlier: the npm client returns timesteps starting AT t0, so lead 0 holds exactly one row per run rather than being empty. 111 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The before/after plot had no severity context: three lines in m3/s with nothing saying whether any of them approaches a flood threshold. Adds two toggleable return-period band sets via the existing rpBandTraces, one per scale, because the lines do not share one: the OBSERVED zones apply to the observed series and to the corrected forecast (correction maps onto the observed distribution), while the SIMULATED zones are the scale the raw forecast lives on. Both start hidden here, and deliberately. Measured the real thresholds for river 210265545 / station 8112: observed 2-yr 81.72 5-yr 182.15 ... 100-yr 456.87 simulated 2-yr 33.05 5-yr 61.62 ... 100-yr 139.76 against a plotted range of roughly 0..16 m3/s. The lowest observed threshold is 5.1x the peak and the lowest simulated 2.1x, so drawing either set by default would stretch the axis to the first band and flatten all three lines into a sliver -- strictly worse than no bands. Visibility is therefore adaptive: a set is shown only when its 2-year threshold is within 1.5x of everything plotted. So that the context is never lost when the bands are off, the subtitle always states it numerically. For this run it reads "peak reaches 14% of the 2-year observed threshold (81.7 m3/s)", which is the actually useful fact: this forecast is nowhere near a flood, corrected or not. Verified against the real thresholds: 12 band traces built, both groups legendonly, subtitle as above. 111 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ports the user's matplotlib figure to plotly so an in-app plot and their own script produce the same chart. Design tokens carried over verbatim -- #2a78d6 / #eb6834, 0.13 band alpha, #fcfcfb surface, y-grid only, no top/right spines -- along with the two touches that make it readable: direct end labels (a coloured dot carries identity, the text stays in ink) and x-range extended past the data to make room for them. Two deliberate departures from the reference. Observations are an optional overlay, because this is a verification tool and without them the plot shows that correction changed the forecast, not whether it moved toward the truth. And gaps render as null rather than 0, so a missing timestep is a break in the line instead of a spike to the floor. Also adds summaryFromMembers, which reduces an ensemble to median plus a percentile envelope. The npm client already returns stats with p20/median/p80 -- exactly the reference script's flow_uncertainty_lower/flow_median/ flow_uncertainty_upper -- but those are the RAW summary only. Any corrected variant has to be recomputed from the corrected members, so the app needs its own reducer rather than reusing the served stats. Tests pin the two fragile parts. `fill: 'tonexty'` fills to the IMMEDIATELY PRECEDING trace, so a band's invisible lower edge and its filled upper edge must stay adjacent -- insert anything between them and the shading silently reaches for the wrong series. And summaryFromMembers takes percentiles ACROSS members at each timestep, not along time, which is the easy thing to get backwards; the test uses a step where those two answers differ. Adding the first test that imports a plot module also exposed a gap in the test tsconfig: it included only "tests", so the ambient declarations in src/types for plotly.js-dist-min and riverforecastsystem were out of project and the transitive import resolved to an implicit any (TS7016), breaking `tsc -b` and the build. Added src/types to its include. 120 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The empirical-CDF method (correct_forecast) is unusable on this reach: 99.4% of the October gauge record falls in one histogram bin, 9 of 16 bins are empty, so the inverse CDF has adjacent entries sharing an identical probability and scipy divides 61.3 by 0. Six of nineteen runs -- exactly the ones that predicted the flood -- map to infinity and get excluded. discharge_transform cannot fail that way. It needs no observed record at all: coefficients are fitted centrally per river and month and published as a zarr store, so there is no local histogram to go empty. Two degree-7 polynomials, one discharge -> log1p(exceedance percentile) and one back, with the percentile clamped to [0, 100]. No division anywhere, hence no infinity. Verified reachable from a browser, which was not obvious for an s3:// URI. The bucket serves Access-Control-Allow-Origin: * over HTTPS, and numcodecs decodes its blosc/zstd chunks in JS -- the decoded October coefficients match the Python package exactly. A cold lookup costs ~8 MB across four chunks and about a second; only the resulting 216 numbers are cached, so it is once per river rather than once per session. Parity is pinned by fixtures generated from the real installed package: 612 probe points spanning every month and both clip branches, plus the user's own 2024-10-25 run end to end -- one of the six the CDF method had to throw away. Max difference 0. The honest part is the diagnostics, because this method trades infinity for saturation rather than escaping the problem. On this river in October the percentile clamps at q = 15.8, so EVERY forecast above ~16 m3/s maps to the same 257.73 m3/s -- the corrected series cannot tell two flood-magnitude forecasts apart. November is worse: non-monotonic, with 40 m3/s mapping below 20 m3/s. transformSeries therefore reports clippedToQmax, atCeiling, atFloor, negativeClamped and a per-month saturation probe including a monotonicity check, so the UI can say this rather than quietly showing a flat line. Writing the probe also turned up an edge case worth pinning: the percentile polynomial rises back above zero exactly at the top of October's Qrange, so that single endpoint escapes the clamp. probeMonth samples inside the saturated region rather than at the boundary. Types live in their own module because the transform maths must not depend on polyfits.ts, which reaches for fetch and IndexedDB -- importing it from a test pulled DOM globals into the node-only test project and broke the build. 133 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds "Bias-corrected — global transform" beside Raw and the existing local-CDF option on Accuracy, Skill summary and Probabilistic. Categorical and Timing stay raw-only for the same reason as before: their dual-threshold classification already absorbs magnitude bias. The two corrected variants fail in opposite ways, so having both is the point. The local CDF is fitted to the uploaded gauge record and dies on a short one -- here it excludes the six runs that predicted the flood, which is why its option is gated behind a selection-bias check. The global transform reads no observations at all, so no run can be excluded and the surviving set cannot be biased; what it can do instead is saturate. On this river in October every discharge above 15.8 m3/s maps to the same 257.73 m3/s, so the corrected series stops distinguishing flood magnitudes -- and November's fit is not even monotonic. GlobalCorrectionBanner states all of that with counts, and `unusable` withholds the variant entirely when essentially everything clamps. All three variants share one comparison grid and, for CRPSS, one climatology built from the observed record. That is what makes the difference between two variants attributable to the forecast rather than to a moved denominator. Coefficients are fetched per river and held as a single river-tagged entry with everything else derived from it. The obvious shape -- separate polyfits/loading/ error states reset at the top of the effect -- sets state synchronously during the effect and trips the cascading-render lint rule; deriving also means a stale river's coefficients can never be read as current. 133 tests passing; lint back to the 2 pre-existing errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Metrics tab was reported as crashing and never loading. A blank tab is the worst possible failure here: the whole React tree unmounts and nothing says which of the two dozen metric blocks threw. Wrapped it in an ErrorBoundary that renders the message and stack in place, so the next occurrence is diagnosable instead of silent, and a failure in one block no longer takes the others down. Two real bugs found while investigating, neither yet confirmed as the reported cause: Typed-array alignment in polyfits.ts. Float64Array and BigInt64Array require an 8-byte-aligned start offset and throw RangeError otherwise, and a decoder is free to return a view partway into a larger buffer. Reading blosc output directly was a latent crash that would fire only on whichever chunk happened to land misaligned -- verified that offset 3 does throw. Now copied to a zero-offset buffer when unaligned. Redundant saturation probing. probeMonth walks 4000 steps of two degree-7 polynomials, and transformSeries called it per month per ENSEMBLE MEMBER per run -- thousands of repetitions of identical work, since saturation is a property of the fitted polynomials and not of the series. Memoised on the fit object via WeakMap. Measured what was ruled out, rather than assuming: the build passes, Blosc.fromConfig is safe at import, correctForecasts over 46 runs against an 86-year retrospective and a 113-year observed record takes 161 ms, and correctForecastsGlobal takes 744 ms. Slow enough to be worth the memoisation, nowhere near enough to explain a hang. Also confirmed skillBars has no temporal dead zone. So the root cause is still unidentified and the boundary exists to catch it. 133 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Metrics tab threw "RangeError: Maximum call stack size exceeded" and rendered nothing. The error boundary added in fb1db5e pointed straight at biasTransfer.ts:17, which is what made this findable at all. Cause: `Math.max(simEdges[last], ...forecastValues)`. forecastValues is every ensemble member at every timestep across ALL runs, and spreading passes each as a separate argument. Measured the limit at ~124,907 elements in V8. The Australian event, at 19 runs, produced 116,280 values and squeaked under it. Both new events use a 31-day window, which the app expands to 46 init dates via the 15-day lookback: 46 x 51 x 120 = 281,520, well past the limit. So this is not about how much observed history was uploaded -- a 33-year record crashes exactly like a 113-year one. It scales with the EVENT WINDOW, and the app has simply never been run with one this long before. Swept the codebase for the same pattern and converted every site that can see a large array to reduce-based maxOf/minOf: biasTransfer (the crash), quantileMap (a month of hourly retrospective is ~64k values, one config change from breaking), leadBuckets, correctForecasts, and skillBars, where I had introduced one myself two commits ago. Also fixes the peak-timing x-axis. Its tick labels used "\n" for the line break, but plotly tick text is mini-HTML, so every label rendered as a truncated "2026-06-05 (" overlapping the axis title. Now <br>, with the run age compacted to "−16 d" and automargin on so the title cannot be drawn through the labels. The regression test asserts both halves: that the old spread form really does throw at 281,520 elements, and that maxOf handles it. 137 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The app drew thirteen kinds of figure and eleven were scatter traces. These are the three forms picked from the study set, each replacing a line chart where the line was the wrong form for the question. Diverging bars for signed error (src/plots/divergingBars.ts). Peak-timing error reduced to one median per initialization, drawn either side of a zero baseline, above the existing box plot rather than replacing it -- the box plot still carries the member spread. When the SIGN is the finding, this asks nothing of the reader: the side answers early-vs-late and the length gives the size. The x range is forced symmetric about zero, because an asymmetric one makes an equal late bar look longer than an early one. Colour is redundant with position on purpose, so nothing is lost in greyscale or to colour-blindness. Dumbbells in the bias-correction section (src/plots/dumbbell.ts). Raw and corrected KGE' per lead as a connected pair, one row per lead, with the connector coloured green where correction helped and red where it hurt and the subtitle counting how many leads improved. Two overlaid lines make the reader match colours across sixteen crossings to answer "did it work"; a dumbbell makes each change a single mark. Rendered for both corrected variants, and it reads the already-computed skill rows rather than recomputing, so it cannot disagree with the bar charts above it. Connectors are drawn per row rather than as one null-separated trace, so hover can report the individual delta. Emphasis colouring in eventVsLead. A Highlight selector switches from sixteen hue-coded leads to one accent line against context grey. The default spends a distinct hue on every lead and forces the reader to carry a colour key while tracing one line through the others; emphasis is the honest answer when the question is about one lead. Context traces are sorted to draw first so the accent is never underneath, and identity stops depending on hue entirely. Also adds the two GEOGLOWS training links to the Overview bias-correction section, per request. Declined from the study set after review: the ridgeline (flips the axes -- the vertical direction would mean both lead day and member count) and the fan chart that was mocked as its replacement. 146 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Several methods that are statistically indefensible on a single event are sound on a multi-decade record, and the app already has the data for them -- it fetches the retrospective and the user uploads the long observed series. This tab uses that pairing and never touches a forecast. On the reference reach (Pascagoula, 750195789) it pairs 11,597 daily values over 32.9 years, against 187 pairs for the event. That is the difference between a method working and not: under a null of zero true resolution the apparent value is about (K-1)*var/n, so ten bins on a few hundred pairs manufactures resolution from noise. Simulated it to confirm the published formula -- at n=17 per lead, pure noise prints RES = 0.126, which is 53% of total uncertainty and roughly twice the real resolution of a well-sampled system. At n=11,597 the floor is negligible. Contents, each chosen because aggregate numbers cannot show it: - headline agreement over the whole record (NSE, KGE' and components, bias, RMSE) - flow duration curves, simulated against observed on log axes, which separate a whole-range offset from a tail-only failure - the conditional-bias curve E[obs|sim] against the 1:1 line, showing WHERE in the flow range the model fails and whether its range is compressed - Murphy's MSE decomposition into REL, RES and var(obs), with a selectable bin count - monthly simulated/observed ratio, which is the view that decides whether the app's monthly bias correction can work at all Two numbers are reported beside the decomposition rather than buried, because without them the terms are not interpretable: the closure error (the split is exact only for discrete forecasts, so continuous discharge leaves a within-bin residual -- 10.1% at 20 bins, 4.5% at 50 on the reference reach) and the resolution noise floor with RES stated as a multiple of it. Verified against Python on the real reach: REL and RES agree to within 0.04%, MSE and var(obs) exactly. The residual is numpy's interpolated quantiles versus index-based edges, not the decomposition. Binning bug found by a test and fixed: a discharge record is full of ties, and tied values collapse adjacent quantile edges. Left in, the collapsed edges swallow every point into one bin -- RES goes to zero and closure blew up to 351% on a two-valued input. Now few distinct values become the bins themselves (exact, no within-bin spread) and many distinct values get deduplicated quantile edges, with the effective count reported as binsUsed. Result on the reference reach: NSE 0.340, KGE' 0.443, bias -46.2%, and RES (100,165) exceeding REL (41,146) -- so over 32 years the model beats climatology, the opposite of its verdict during the June 2026 flood. Same reach, same model, and now the event can be read against a baseline instead of in isolation. 160 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Long record" described the sample; "Retrospective evaluation" names what is actually being evaluated, and matches the vocabulary used everywhere else in the app and in the RFS data products. Renamed in the tab list, the error-boundary label, and both section headings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reverts 71ce082 and 0167636. The tool evaluates forecasts; a tab scoring the retrospective against observations measured the hydrological model with perfect meteorology, which is a different question and out of scope. Deleted RetrospectiveTab, retrospectiveEval, and the conditionalBias and flowDuration plots, plus their tests, and unwired the tab from App. Checked dependencies first: nothing outside that set imported any of them. Three things deliberately kept, because they are used by the forecast path: arrayStats (seven modules depend on it -- it is the fix for the Math.max spread crash), and the dumbbell and divergingBars plots, which MetricsTab uses for the bias-correction comparison and peak-timing error. 146 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Audited it against the current behaviour. No stale references to removed features, but three real gaps: The global transform was absent entirely -- the Overview described bias correction as if the local CDF were the only method, while the app has offered two since 3f89858. Added a section covering both, why they fail in opposite ways (the local one can map to infinity and exclude whole runs; the global one saturates and is not guaranteed monotonic), and the point that which to prefer is visible in the diagnostics rather than decidable in advance. "A bias-corrected variant" is now plural to match. The MCC and HSS sections asserted properties we have since measured to be false or misleading. Added a caution block covering three things: both scores move with the uploaded window length -- a forecast systematically one category low goes from -0.50 to +0.35 on padding alone, which is the opposite verdict for identical performance; the two are not independent checks, since the formulae printed directly above each other share the numerator N*c - sum(t*p) and differ only in the denominator; and the multi-category MCC has no floor of -1, so a negative value has no fixed reference. Also recorded that CSI and F1 are the only exactly window-invariant options, and that EDI/SEDI are NOT -- padding drives them toward 1 regardless of skill, which is worse than MCC. The 31-day event window cap was undocumented despite being enforced on the Forecast tab. Added it, with the two reasons: cost scales with the 46 runs it implies, and a window much longer than the flood dilutes the categorical scores through exactly the mechanism the MCC caution describes. 146 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two related additions to the Setup tab. Window guidance. The right window is hydrograph-shaped, not a fixed number of days: the two events verified with this app wanted 5 days before the peak and 25 after, and 16 before and 14 after. A fixed rule would have clipped one of them. suggestEventWindow opens at the last minimum before the rise and closes once flow is back within 30% of it, and the panel explains the three rules behind that -- bracket the event at comparable flow; cut from the FRONT when the 31-day cap bites, because losing the falling limb distorts every volume and timing metric while losing pre-event days costs only a little lead coverage; and prefer longer otherwise, since pairs per lead equal the window length in days. Measured before writing the advice, so it says what the numbers support. On the Pascagoula event, pairs/lead is exactly the window length, and the categorical base rate falls from 87.5% at 8 days to 25.8% at 31 -- but the true long-run rate is 1.05%, so it stays 8-80x inflated at EVERY length under the cap. Shortening the window therefore cannot fix categorical dilution, and the panel deliberately does not advise it; that has to be read alongside the pair count instead. Event data from the historical record. A multi-decade upload already contains the flood, so a mode was added that takes a date, finds the peak within 10 days of it, and cuts the window with the same rule. Removes the need to upload the same data twice. Both paths are kept rather than one replacing the other, because they are not equivalent and the difference is stated where the choice is made: a historical record is usually daily, and the comparison grid is the coarser of the two sides, so an extracted event caps peak timing at 24 h however well the window is chosen. Uploading a sub-daily file is the only way to do better, and extractEvent surfaces that as a cadence caveat rather than leaving it to be discovered. 159 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The introduction laid out four metric families and never said that two of them are affected by something which is not a forecast error at all -- a reach where the global model runs 40% low scores badly on magnitude however well it caught the event. Bias correction only appeared much further down, so a reader meeting the framework had no idea the corrected variants existed or why. Added a paragraph after the family table: what the bias is, which three blocks offer corrected views, that there are two methods with different failure modes, a pointer to the section that covers them, and why Categorical and Timing need no correction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
These files usually already live in a bucket, so downloading them only to re-upload is pointless. Both paths run through the same parser, so column detection and the skipped count behave identically however the data arrived. The constraint is CORS, and it is stated up front rather than discovered through a failure. A browser can only read a cross-origin response when the host sends Access-Control-Allow-Origin, and nothing the page does can work around it. Checked real endpoints: geoglows-v2 and noaa-nwm-pds both send it, so AWS Open Data buckets generally work; a private bucket generally does not. On private objects: a presigned URL is the right answer, and needs no change here since it is just an HTTPS URL with the signature in the query string. But signing and CORS are enforced separately -- signing satisfies S3, CORS is enforced by the browser -- so a private bucket needs BOTH, and the error text says so. No credential entry was added: handling a long-lived AWS secret in a browser page is a bad idea on its own terms, and it would not solve CORS anyway. Two details that matter for presigned URLs specifically. Error messages show only host and path, never the query string, because X-Amz-Signature is a short-lived credential that should not end up in a screenshot or a pasted issue. And the loaded-file label uses the object key rather than the full URL, for the same reason. A blocked request surfaces as a bare TypeError with no status and no detail -- the browser hides the reason deliberately -- so the failure message names the likely cause, says it is a guess, and points at the fallback that always works. Also catches the case where a wrong key returns an HTML or XML error page with HTTP 200, which would otherwise produce a baffling column error. 159 tests passing; lint back to the 2 pre-existing errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The URL option already explained both constraints, but as 0.8rem grey prose under the input -- which is where text goes to be skipped. A user who misses it pastes a perfectly correct URL, gets an opaque failure, and has no way to guess why, because the browser deliberately hides the reason. Now a bordered notice shown whenever the URL source is selected, with the two requirements as separate numbered points rather than one paragraph: the host must send Access-Control-Allow-Origin, and a private object needs a presigned URL -- with the trap stated explicitly, that a presigned URL does NOT remove the first requirement, since signing satisfies S3 while CORS is enforced by the browser. Names the buckets actually verified to work, so the claim is checkable. The footer says a blocked request is indistinguishable from an unreachable host, and points at the fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same diverging-bar form as the per-initialization version, grouped by lead day instead, built from app.peakTimingDistribution so it cannot disagree with the box plot it sits above. This is the more interpretable of the two, and the note says why. The per-initialization chart carries a censoring artifact: a run started long before the peak can only place that peak inside its own 15-day horizon, so it is forced early, while a run started on the peak day can only place it at or after, so it is forced late. On the reference event that produces a clean-looking sign flip -- -58.5 h at the earliest init, +54 h at the latest -- most of which is geometry rather than skill. Grouping by lead compares forecasts at a consistent horizon, so a systematic lag reads as a real lag. Unscored leads are distinguished rather than lumped: "no overlapping timesteps" when the pair count is zero, "no member timed a peak" when there were pairs but no member produced a peak inside the search window. Those have different causes and different fixes. 159 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bias correction leans on the dual-threshold design to explain why Categorical and Timing need no corrected variant, but that design is not set out until two sections later. A reader meeting the term there has to take it on trust or go hunting. Both earlier mentions -- the introduction and the bias-correction section -- now say where the explanation is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Corrected on the point that discharge_transform IS SABER. I had inferred otherwise from the existence of a separate sfdc_bias_correction function and written "neither of these is SABER" into the Overview. The evidence supports the correction: discharge_transform's own docstring calls its inputs "transformation weights", the store is transformers/polyfits.zarr, and geoglows.bias.sfdc returns a scalar per (month, exceedance probability) which is what the polynomials encode. The mistake mattered because it made the trade-off wrong. I had described the global option as using "no observations at all". SABER uses gauge observations -- just not necessarily YOURS. It compares curves at gauged reaches, clusters watersheds by flow behaviour so ungauged reaches can borrow from gauged ones, and publishes the result centrally. So the real trade is not observations versus none; it is whose observations, and whether a centrally clustered reference describes this river. That also explains something previously left as a curiosity: a SABER ceiling well below the user's own record maximum belongs to the reference curve that was fitted, not to their gauge. Checked the scalars directly, which surfaced a case worth documenting. For Wisconsin at Muscoda every one of the 1212 scalars is exactly 1.0 -- SABER has no correction for that reach, so the transform is an identity and any movement is polynomial fitting error around a no-op. Pascagoula runs 0.065 to 2.484 (median 0.476) and the Australian reach 0.000 to 0.539 (median 0.063), both real corrections. The Overview now says to check for this. Also records that the training material describes SABER as experimental and not applied to the forecast data end users receive, so the downloaded forecasts are uncorrected and choosing this variant is what applies it. Renamed the variant from "global transform" to "SABER" in the selectors, plot titles and banner. 159 tests passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both links resolved through Crossref rather than pasted raw, so each carries authors, year, title and journal instead of a bare URL, and points at the DOI rather than a publisher-specific path that can rot: - SABER: Hales et al. (2022), "SABER: A Model-Agnostic Postprocessor for Bias Correcting Discharge from Large Hydrologic Models", Hydrology 9(7) 113. - Local method: Sanchez Lozano et al. (2025), "Historical simulation performance evaluation and monthly flow duration curve quantile-mapping (MFDC-QM) of the GEOGLOWS ECMWF streamflow hydrologic model", Environmental Modelling & Software 183 106235. The second confirms what the local method is called: the training material names MFDC-QM, and that is the paper behind it, so the section now uses the term. Applied on top of the user's own edits to this section, which had renamed the first method and rewritten its description. Also removed a doubled quote mark in the SABER line -- a typing slip, not a deliberate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…aught The by-run panel's note is headed "What is not in these boxes" and did not mention emptyRuns, unusableRuns or unusableMembers — two of which I added an hour ago specifically to close the accounting, then never displayed. The data added up; the disclosure on screen did not. All three are now named. "The longest lead day reaches only N pairs" named the wrong lead. N is feasibility.achievable, a MAX over leads, and a run coarsens across its horizon — so the longest lead is the WORST-sampled, not the best. Now says "best-sampled", which is what the number is. The remedy did not branch on grid.limitedBy, which was in scope. When the forecasts are the coarser side, uploading a finer gauge record cannot add a single pair, because chooseGrid takes the coarser of the two — yet that was the advice given. It now offers the one that works: a longer event window, which pools more initializations into every lead bucket. "Lead 1 is representative: a full day of the run's native output" was false and contradicted the pairsPerLead docblock. Lead 1 is the FINEST day, and choosing the grid from it is deliberate — the cost is that later leads land on a grid finer than their own publishing interval. OverviewTab: the paragraph explaining why that panel can disagree with the metrics page sat in the `else` branch, so it rendered only while there was no data and vanished the moment the data it warns about loaded. Moved out of the conditional, and rewritten to say what actually differs: whole-run median spacing here, lead 1 there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…p is not read as bias A perfect forecast reported a unanimous -3.0 h at leads 8-15 and 0.0 h at leads 1-7. Measured, not inferred: 20 identical-shape runs against 3-hourly observations, with the run coarsening to 6-hourly after day 7. All members agreed, so the box was tight and the step clean — it read as real early bias appearing at lead 8, and was entirely the true peak at 03:00 being unrepresentable on a 6-hourly lattice. The cause is that a run does not publish at one spacing. All 51 members share a single time index, but that index is finer early than late, and the argmax can only land on a sample that exists. The observation side meanwhile sits on the finer comparison grid, so the two are quantized differently. Not suppressed, drawn. Each timing distribution now carries its own per-lead resolution and the box panels shade a band at +-that; the diverging bars draw a bar hollow when its median is within the row's own spacing. A reader still sees the step — it is a real fact about the data — but cannot mistake it for a measurement. Threshold crossing gets the same band, where it matters more: a crossing can only be detected on a sample that exists, so the coarse leads bias one-sided LATE rather than scattering. peakTimingByRun had no grid escape at all, reading raw run.time, so its rows now carry the median spacing of the span actually searched. The test asserts the property that makes the band sufficient: on a perfect forecast every reported Δt is non-zero somewhere AND within its own lead's spacing everywhere. It also asserts the resolution really varies, or it would prove nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ird of it Third attempt at this, and the first two fixed the wrong thing. I twice adjusted the prose measure, which was never the problem: text wrapping at 46rem is correct. What looked broken was the CONTAINER — App.tsx set maxWidth 1800 on <main>, so a section box stretched the full screen and its prose filled 42% of it. A text column at 42% of its own box does not read as a text column. <main> is now 1120, which puts prose at about 70% of the box — a normal document column. Every plot is width:100%, so they follow; nothing had a hardcoded width. The two flex rows with minWidth constraints (contingency matrix beside its plot, the 620px score table) still fit inside 1056px of content. The Compute buttons were the other half. They sit in flex columns, whose default align-items is `stretch`, so a button rendered as a full-width bar across the whole block — the widest element on the page for the smallest action on it. Now sized to their labels, in all three tabs, so a future flex column cannot bring it back. Also in this commit, the Temporal resolution section of the Overview, rewritten: - New subsection on the forecast changing spacing across its own horizon, which is the fact the rest of the page kept implying and never stated: the grid comes from lead 1, later leads carry fewer pairs, and timing is quantised differently by lead. - Two corrections the audit caught: NSE rides the bin-MEAN grid and was missing from that list, and the bin-summary selector drives the TIMING family too, not just the categorical one. - Bin summary cut from 70 lines to 54 by merging the two "which to choose" paragraphs. - The ensemble-reduction detail moved to the Contingency matrix section, where the selector actually lives, and corrected: six statistics are offered, not the four it claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…styles Third time I have claimed this was fixed, and the first two attempts each fixed a subset. What was actually wrong, in order of discovery: 1. named styles: some had a measure, some did not — fixed in c47f87a 2. the page container was 1800px wide, so a box spanned the screen while its text filled 42% of it — fixed in f5637ba 3. INLINE `<p style={{…}}>` elements never had one at all, and OverviewTab had no measure on anything, named or inline — this commit (3) is what the screenshot showed: a block's description wrapping at 736px with the sentence directly beneath it running to 1005px. Same box, two widths, because one used a named style and the other an inline one. 53 elements across the four tab components now carry it: 25 inline in MetricsTab, 10 in OverviewTab including its p/ul/caution/pMono named styles, 8 in SetupTab, 7 in ForecastTab. Nothing left unmeasured — asserted by grep, not by eye, which is what I should have done the first time. PROSE_MAX moves to src/prose.ts so there is one value rather than one per file, since per-file is how it drifted. Deliberately still NOT applied to plots, tables or the section boxes: those need the content width, and the page container is what keeps them sane. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tually does
Second half of the section. The two longest bullets were essays inside list
items — 16 and 19 lines, each with a <br/>-separated second argument — so both
are promoted to their own subsections, and the floating-point digression in the
first is cut to the clause that carries the point ("decided by floating-point
luck").
The ceiling section now states the reason nothing is excluded, which the old text
asserted without arguing: a forecast only exceeds the simulated maximum when it
predicts something extreme, so dropping those runs preferentially deletes the
ones that saw the event coming and leaves a corrected score computed mostly from
the runs that missed it. That is a stronger reason than reference fidelity alone.
New subsection on SABER's own limits, none of which the page mentioned:
- it saturates at BOTH ends, and each is reported separately, with the high end
named explicitly because that is the one that flattens floods
- each figure is measured inside its own saturated region, so the value quoted
is one that inputs there really produce
- the fits are not guaranteed monotonic
- inputs are clipped to the fitted range at both ends, and both are counted
- three distinct withholding rules: no published transformer, NaN coefficients
in a month the event touches (no tolerance threshold, and why), and
saturation of essentially everything
Also added: values already missing in the download pass through uncorrected, and
the banner counts them — a gap is genuine, since every member of a run shares one
time index.
notePara removed from this file; both its users were the promoted bullets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The user asked me not to make up information, and was right to. Five figures on
this page were quoted as measurements with nothing behind them — they came from
scratch scripts that no longer exist, and I had added another one today from
research I never verified.
Audited every measurement-shaped number in the page against the repo. Removed,
keeping the qualitative claim in each case:
- "MCC 0.45 to 0.82" under padding, and "-0.50 to +0.35"
- "the correlation between their gap and |log bias| is 0.72"
- "CSI reads about 0.08 to 0.12 higher"
- "0.0102 whole-record against 0.0419 in season"
- the climatology triple "-0.11 / -0.05 / +0.67"
- "15,586 gauges" (removed earlier in the rewrite)
Re-derived the one that was load-bearing instead of cutting it. The claim that
MCC and HSS cannot disagree on sign is what tells a reader that "MCC much lower
than HSS" cannot mean skill earned on normal flow — so it needed to be true, not
remembered. tests/lib/mccHssOrdering.test.ts is a deterministic 60,000-matrix
sweep: 0 order violations, 0 cases of MCC below HSS while both positive. The page
now says the sweep is kept in the repository.
Every remaining measurement-shaped figure in the page traces to code or a test.
Also in this pass:
- "Where the published method and the code disagree" renamed and rewritten. The
heading read as though THIS APP departed from the method; it does not. What is
verifiable first-hand is that the port matches geoglows 2.2.0 bit for bit
across 19 cases, that the reference's own outputs contain the infinities, and
that the published evaluation scores retrospective simulation rather than
forecasts. The version-history and dissertation claims are cut: I could not
confirm them from primary sources, and a search for the "deviation factor"
rule surfaced a different paper's method instead.
- SABER's limits now state their provenance explicitly: measured by probing the
published coefficients and reading discharge_transform, NOT taken from
Hales et al. (2022), which describes the fitting method rather than the
fitted transforms' edge behaviour.
- Peak timing: the exclusion rules, the plateau-first-sample rule and the
resolution band, none of which the page mentioned.
- CRPSS: retracts "share one implementation" — they share the season rule, not
the aggregation — and records the daily-floor inflation.
- KGE': why a member can appear in the beta panel and not the KGE' one.
- Intro: CSI moved out of the per-member list, since it is pooled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…gure Same provenance sweep as the Overview, applied to the chart explanations, which I had not checked. The categorical-scores note claimed MCC and HSS "correlate at 0.99 across thousands of contingency matrices" with nothing behind it. Measured on the deterministic 60,000-matrix sweep already in tests/lib/mccHssOrdering.test.ts: 0.9944. So the figure was right; it is now reproducible, asserted to three places, and the note says where the sweep lives. The CSI note claimed CSI "reads about 0.08 to 0.12 higher on a severe event" than the multi-category scores. No source, and unlike the correlation there is no cheap way to derive it, since it depends on the event. Cut to the qualitative claim, which is what the paragraph needs: collapsing to "at or above the 2-year level" is an easier question, so CSI reads higher for that reason alone. Audited every measurement-shaped figure in the metrics UI the same way. The rest trace to code or tests; the remaining matches are a unit constant (35.31 ft3/s per m3/s) and CSS alpha values. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ong twice Two findings from auditing the chart explanations. The 800-fold drift quartet (CSI 0.003, RPSS 0.028, MCC 0.070, HSS 0.074) is prose. I "verified" it earlier by finding the numbers in rps.ts — but that is a comment I had written, citing nothing. Circular, and exactly the failure mode already corrected once today. No test derives these values. Removed from all six places they appeared: three chart notes, two code comments and the Overview. The qualitative claim stands and is what the paragraphs need — CSI does not climb with window length and MCC/HSS do. Same for "its ability to rank a known-better forecast measures 0.576, a coin flip", which appeared in three places and cannot be derived without fixing the event. Replaced with the mechanism, which is the actual argument: the median collapses at high thresholds because most members produce the same degenerate table. Separately, the figure counts in the Compute-button justifications were wrong, because my grep for `<Plot` also matched `<PlotNote` and roughly doubled every count. Real numbers: Categorical 4, Timing 3, Accuracy 6, Probabilistic 2, Bias correction 3 — 18 in total, not the 38 I have been quoting. The accuracy comment said TWELVE where it is six; the bias one said SEVEN where it is three. The conclusion is unchanged — nine unbuttoned figures were still worth gating — but the numbers were not checked, and the PR description needs the same correction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…read "8/1/1977 0:00" — what Excel writes by default on a US locale — was rejected. The ISO path turns a space into T and appends Z, so it became "8/1/1977T0:00Z", which is not a date; every row was skipped and the file was refused with a message that never mentioned dates. Slash dates now parse. The hard part is that 8/1/1977 is 1 August in the US convention and 8 January nearly everywhere else, and Excel writes the host machine's locale — so the same spreadsheet exports differently on different computers, and the format cannot be configured, only detected. detectDateOrder reads the WHOLE file before parsing any row. A component above 12 settles it: 25/12 can only be D/M, 12/25 can only be M/D. One row cannot settle it, and a per-row guess would read one file two different ways depending on where the days happened to exceed 12. When nothing settles it — a record of month-starts, or a short window early in the month — the series still loads, read as M/D/Y, but the uploader says so and prints the resulting date span. This must not be silent: a record read the wrong way round is not visibly broken, it just misdates every value, which moves the season a climatology is built from and the window an event is scored over. The same warning fires when the file is internally inconsistent, where neither reading works for every row. Two details worth stating. A component above 12 overrides the file-level order, so a stray 25/12 in an M/D file still lands correctly. And impossible dates are rejected rather than rolled over — Date.UTC(1977, 3, 31) silently becomes 1 May, so 4/31/1977 returns null instead of a wrong date. The upload summary now also prints the first and last date parsed, which is the cheapest way for a reader to notice a misread file. 13 tests, including every ISO form that worked before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ot have From the plot-note audit. Each was verified against the code before changing. The band on the wrong chart, which was mine from two hours ago. The by-lead peak timing note opened "The grey band is the lead's own sampling resolution" — but that panel is divergingBarsFigure, which draws no band. The bars got the HOLLOW treatment; the band is a distributionVsLead feature. I had the two backwards. Worse, peak timing has no box-plot panel at all, so the band never renders for it — hollow is the entire signal there, and the note now says so. Bias-correction run list: said the local map "drops runs whose mapping ran to infinity" and pointed at a selection-bias banner. That exclusion was deleted commits ago and selectionBias is hardcoded null, so the banner can never appear. The real reasons a run is dropped are no timesteps, or no usable mapping for its month. Added why an above-range run is deliberately kept. Correlation panel: "no bias correction would fix it" said of low r. But r IS re-scored under each variant — quantile mapping is a per-month transform and does move it — and where a correction saturates a member flat, r stops existing for that member rather than improving, so the member leaves the box instead of scoring badly in it. Band ladder: presented as "the published KGE' classification" including the -0.41 floor. Thiemig et al. publish four bands with a single Very poor at or below 0; the five-band split is this app's extension. The Overview already said so and the chart contradicted it. Return-period zones: "yellow for 2-year through purple for the rarest category in play". Purple exists only at 50 and 100 years, so on a 10-year event the topmost band is red. Comparison table: "1.4 is as wrong as 0.7". improvement() is linear in |value - 1|, so 1.4's equal is 0.6. Contradicted by an existing test. Cadence breakpoint: the notes said RFS publishes "3-hourly for the first week" and I nearly replaced it with "ten days" — but BOTH numbers come from synthetic runs I constructed, not from observed RFS output. Neither is asserted now; the text says the spacing switches partway through the horizon, which is the part that is actually known. The test fixture's docstring no longer claims its 7-day breakpoint is "as RFS publishes". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Taken from the shared Google Doc, applied section by section rather than pasted, so the parts that are components rather than prose survive: the live ResolutionNotice, both tables, the four external links, the formula blocks and the caution boxes. What the rewrite changes: - Introduction gains a short "what you need to use this app" paragraph, left with the author's own (EDIT ME) marker. - The CSI bullet is dropped from the ensemble-handling list. That list is about how a metric treats the 51 members, and the bullet led with window-invariance, which is a different subject; CSI's own section already covers both. - The spread bullet is rewritten to say what spread is and which failure actually matters — a narrow spread around the wrong answer, not a wide one. - Temporal resolution: "The rule" becomes "Handling mismatched resolutions" and "The forecast is not one resolution" becomes "RFS Forecast", both shortened. - The bin-summary explanation loses its paragraph labels, its code identifier and its two worked figures, which the author found more confusing than helpful. - Bias correction is cut hard, and deliberately: the Overview says that bias correction exists and which metrics use it, and is not the documentation for it. Four subsections go — the published-evaluation scope argument, the low-flow outcomes, SABER's own limits, and the shared limits. The low-flow behaviour and the clamped-negative count already appear in the correction banner, at the point where they change a number, which is better placement than here. The bias-correction paragraph also moves out of "Structure of the verification framework", since that section is about the four families. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The repo had none, which meant the setup steps and the CSV format lived only in whatever email or conversation happened to carry them. Covers what the app is, how to run it, and the four tabs in the order they are meant to be used. Two things get more space than their length suggests, because both are where a new user actually gets stuck: - The CSV format. Columns are taken by POSITION, not by name, which is not guessable. Dates may now be ISO or slash-formatted, and the slash case carries the month/day ambiguity, so the note says to check the range the app reports before trusting anything. - That a long historical record is not optional in practice: it is what the return-period thresholds are fitted to, and it gates the skill scores and the local bias correction. Every factual claim in it was checked against the code rather than written from memory — the 9-digit reach id, the 31-day cap and its 46 runs, the 15-day lookback, five metric blocks, 360 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Says what the two inputs are — a nine-digit reach ID and observed discharge — and adds the part the placeholder left out: why the historical record needs to be long. It is what the return-period thresholds are fitted to, and it gates the skill scores and the local bias correction, so uploading a short one is the commonest way to end up looking at blank panels with no explanation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five subsections become four, and the surviving prose says each thing once. Removed: the Consequences bullet list, which restated the three paragraphs above it; the "23 values per real measurement, 24-fold" arithmetic, where the claim survives without the worked number; the three RFS bullets, folded into one paragraph; a paragraph under Your current data that duplicated the section's own opening; and the note explaining why that panel's cadence reading can differ from the metrics page's. That last removal is the only one that loses something. The two readings do legitimately differ on a run whose spacing changes — this panel takes the median over a whole run, the metrics take lead 1 — and a reader who notices now has no explanation. Cut on the author's call; one sentence would restore it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It was three paragraphs documenting how MFDC-QM behaves above the fitted range — the flat CDF top, the finite-versus-Infinity coin flip, and the argument for not excluding those runs. That is documentation of the method, and this page is not that; it says bias correction exists and which metrics use it. Nothing is lost operationally. The correction banner counts these values where they occur, next to the numbers they affect, which is where a reader meets them anyway. Renamed the surviving subsection from "The local FDC mapping, in detail" to "Learning more", since after the cut it is two links to the GEOGLOWS training material and no longer contains any detail to promise. Bias correction is now 382 words across three subsections. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four described behaviour the code does not have: - "the mapping is the same at every lead" is true of the local map, which picks one curve per run, but false of SABER: its coefficients are per calendar MONTH and applied per timestep, so a run whose horizon crosses a month boundary does switch curves partway along. The note told readers any lead-dependence could not be the correction. - "where grey and blue coincide the raw value was kept" is the local map's behaviour. SABER writes a gap instead, and the panel is shared by both. - "Each set stays hidden when its threshold is far above everything plotted" — only the observed set is range-tested; the simulated set always starts hidden. - "matching the black median line on the box plots above" — the arithmetic does match, but this block has TWO independent variant selectors, so the bar and the box can be showing different variants. NSE also has no box plot above it. Two pointed somewhere unhelpful. One referred to a reason "given under the per-initialization chart below", which that note does not contain and which is not rendered on the branch where the pointer appears — so the reason was nowhere on the page; it is now stated inline. The other said a note "says which button fills which" when, since the accuracy distributions became memos, only CRPS and CRPSS still need a button and the note says the rest need none. One overstated: threshold crossing being "biased late" was asserted as a result, but nothing measured it and there is no test for that module. Demoted to the mechanism, which is sound on its own — a crossing can only be detected at or after the true one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The banners and notes were mixing two different jobs: reporting what happened to the user's data, and teaching the method's internals. The second is not this app's job — the papers are linked on the Overview tab and do it properly. Cut the mechanism, kept the diagnostic. The ceiling bullet no longer explains that the simulated distribution is flat and the mapping has no inverse; it says how many values landed there, that magnitudes above that point are indistinguishable, and what that means for reading the score. The infinity case no longer explains cumulative sums disagreeing in their last bits. The low-resolution month no longer explains histogram binning. The non-monotonic bullet no longer explains that these are degree-7 polynomial fits. Also removed a code identifier that had reached the user interface (discharge_transform), and the "monthly quantile mapping" framing in the local banner's header, which now names its inputs instead of its technique. The section description points at the Overview's paper links for anyone who wants the method itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removed from the Overview, on the author's call: "So a SABER ceiling can sit well below your record's maximum, because it belongs to the reference curve rather than to your gauge." The paragraph still makes its point — both methods use gauge data, the difference is whose — without explaining what the method does at its edges. Also cut the peak-timing-by-lead chart note from 527 words to 168. It had grown into a restatement of the Overview: the whole sampling-resolution argument, the perfect-forecast measurement, the exclusion rationale and the axis-scaling history. What a reader needs at the chart is how to read the pattern — which side is early, what the two whiskers mean, what hollow signifies, and to check the member count. The reasoning stays on the Overview where it belongs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cut two sentences: the several-fold disagreement between baselines, and the note that the window wraps across New Year. The first was an unquantified aside, and the second is an implementation detail no reader acts on. What remains is the part that matters — the reference is season-restricted so that beating it is not partly a reward for knowing what month it is. Also fixed the stray indentation this bullet had been carrying. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cut the justification for withholding and the note that RPS and CRPS are still reported. The bullet now states the rule: no historical upload or fewer than 30 in-season values means no skill score. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cut the mechanism and the measured figure: nothing in the window crossing the lowest threshold means climatology is right by default, and RPSS is withheld. The reader does not need the ratio's behaviour or what it did to the axis. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The metrics section was 4,008 words across twelve subsections, each titled "Family — Metric" — 68% of the page and the only part nobody had cut. It is now four sections, one per family, at 1,857 words. The prefixes go with the split. MCC and HSS merge into one subsection. They share a numerator and differ only in denominator, so they cannot disagree on sign, and their two caution boxes were saying overlapping things across 657 words. Deduplication that only showed up once the families sat side by side: the RPSS/CRPSS reference rule was written out twice in full and now appears once, under RPS, with the probabilistic section keeping only what is genuinely CRPS-specific. "Withheld rather than estimated" appeared three times in near-identical words. Sign conventions were re-explained per metric and are now stated once where they first apply. Cut outright: the algebraic derivation of the -0.41 benchmark, which is not what a reader needs to interpret a coloured bar, replaced by what the benchmark means. Two things checked on the way in, both flagged by the drafting pass. The old page claimed CRPSS "shares one implementation with RPSS" and then corrected itself in a later paragraph; both are gone rather than one surviving. And the draft had reintroduced the New Year wrap and the "ratio explodes" clause, which the author had just deleted — neither made it in. Overview total: 3,679 words, from about 7,000 this morning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two sentences under each of the four family headings, so a reader arriving at a section knows the question before meeting the first formula. Each says what the family measures and what distinguishes it from the others: - Categorical treats flow as a category rather than a number, which is the form a warning decision takes, and is scored raw because the dual thresholds already absorb magnitude bias. - Timing ignores magnitude entirely, so timing and magnitude can fail independently instead of being hidden inside one combined score. - Accuracy compares discharge directly, making it the only family that measures bias and the only one a correction changes. - Probabilistic reads the 51 members as one distribution rather than scoring them separately, so honest uncertainty can beat confident error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pMono had no whiteSpace rule, so every {'\n'} in a formula block collapsed to a
space and all three rendered as one run-on line. Added pre-wrap — wrap rather
than pre, so a long formula still breaks on a narrow screen instead of
overflowing the container.
The threshold-scores block is reformatted with one score per line and each
symbol on its own line, which is what it was trying to be. The MCC/HSS and RPS
blocks are now built the same explicit way rather than relying on JSX's
whitespace handling around an embedded newline, and their equals signs line up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It said "its own panel" and then "no shared axis with MCC and HSS" — the same layout fact twice, and one the reader can see for themselves. Kept the part that is not visible: do not read CSI against MCC or HSS. The scales look alike but CSI answers an easier question, so it reads higher for that reason alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The list described r, beta and gamma in words while every other metric on the page showed its formula. Added a block above the list, checked against the implementation rather than written from the usual textbook form: r = cov(f, o) / (σ_f · σ_o) β = μ_f / μ_o γ = (σ_f / μ_f) / (σ_o / μ_o) Also noted on the gamma bullet that using CV rather than σ alone is the thing that makes this KGE-prime rather than the original KGE — the page names the prime everywhere and never said where it comes from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
They had reached 4,218 words — longer than the whole Overview page — because each one had grown into a restatement of the reasoning that belongs there. Now 1,065 words across all twenty, none longer than 85. Every note now answers only: what is plotted, which direction is better, and what the markings mean. Anything explaining WHY a metric behaves as it does, or what its limits are, is on the Overview and is not repeated here. Removed across the twenty: the sampling-resolution argument and the perfect-forecast measurement behind hollow bars, the member-exclusion rationale, the window-invariance derivation, the reference-construction rules, the axis-scaling history, and the reasons CSI sits apart from MCC and HSS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each of the five blocks carried a paragraph naming its metrics and explaining what the family is for. That is Overview material — the Overview now opens each family with exactly that, and the headings inside each block already say what it contains. The description prop had no callers left, so it is gone too, along with the style it used. The metrics page now carries 1,065 words of explanatory text, all of it instructions for reading a specific chart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
index.css shipped a prefers-color-scheme dark block from the Vite starter template in the initial commit, and no component ever used the variables it sets. Every colour in the four tab components is a hardcoded light-theme hex. The result was an app that responds to the preference just enough to break: on a dark-mode machine the page background went near-black while the cards kept their hardcoded white and the text its hardcoded dark grey. Invisible to anyone developing on a light-mode machine, and not caught by tests or lint. Now declares color-scheme: light and drops the unused dark palette, so it renders the same for every viewer. Declaring it rather than leaving it unset also keeps browser-drawn form controls and scrollbars light, instead of dark chrome around a light page. Real dark-mode support is a separate task: it means routing a few hundred inline colours through the variables that already exist, and re-theming every Plotly figure, which sets its own background and font colours. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous comment described the single theme as a consequence of the components hardcoding light colours, and the commit message called dark mode "a separate task" — both of which read as an invitation to add it later. It is not planned: the app is light only by choice. Says so at the declaration, along with the fact that the removed prefers-color-scheme block came from the starter template rather than from anyone wanting it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overhauls the verification metrics: what they measure, how they are presented, and what the app tells you about its own limits.
Layout and presentation
<main>was 1800px, so a section box spanned the screen while its prose wrapped at its reading measure — text filling ~42% of its own container. Now 1120px, which puts prose at ~70% of the box. Every plot iswidth: 100%and follows.PROSE_MAX(src/prose.ts) is applied to all 53 prose elements across the four tabs. It had been applied to some named styles and no inline ones, so two paragraphs in one box could wrap at different widths.align-items: stretchrendered each as a full-width bar.Correctness
[]against the old code and[132]against the new.leadMemberScores.ts).references.ts). CRPSS's reference was floored at daily resolution while CRPS was scored sub-daily; a narrower reference is easier to beat, inflating CRPSS by up to 0.09 near the climatological median — most where the verdict is marginal.computeCsi(m, 0)returned 1.0; mismatched threshold counts gave RPSS 1.0000 for calling ≥5yr on a ≥10yr day. Plus two crashes and one fidelity fix (±Infinitywas skippingclip(lower=0), deviating from the pandas reference).Disclosure
Several diagnostics existed but were unreachable or wrong: SABER reported every withholding as "transform saturates" and pointed at a banner that only renders once SABER is selected — which this reason disables. Peak timing's exclusion counts were computed and never displayed. The pairs-per-lead banner printed the maximum across leads as though it were typical, overstating late leads 2×. The comparison table showed dashes with no explanation of which button fills which row.
Overview page
Rewritten section by section. Adds the cadence-coarsening story the rest of the page kept implying, the peak-timing exclusion rules, and SABER's limits — and splits Bias correction into eight subsections from two essay-length bullets.
Provenance: five figures were removed because they came from scratch scripts that no longer exist and could not be re-derived. The one that was load-bearing — that MCC and HSS cannot disagree on sign — was re-measured instead and pinned as
tests/lib/mccHssOrdering.test.ts(60,000 matrices, 0 violations). Every remaining measurement-shaped figure in the page traces to code or a test. SABER's limits now state that they were measured by probing the published coefficients rather than taken from Hales et al. (2022).For the reviewer
tsc -bclean,vite buildclean, lint 0/0.maxbin summary, an identical perfect forecast reports a peak-day value of 390.00 at leads 1–7 and 294.10 at leads 8–15 when the crest falls on an odd 3-hour instant — about half of crest phases; (2) the CSI panel's hollow "too few events" marker fires at long lead because the bucket holds 4 timestamps instead of 8 for the same flood. Neither affects the default view: the median summary avoids (1), and (2) changes only a marker.-0lead-bucket edge case (a pre-t0timestep landing in lead 0) is left unfixed on purpose — numpy'sceilbehaves identically, and reference fidelity is the point of the app. It is flagged in a comment at the site.🤖 Generated with Claude Code