fix(scoring): prove the persistence baseline only sees the past, withhold it where it degenerates - #586
Conversation
Add leakage tests for the persistence baseline in scripts/score_releases.jl. A baseline built from a history that runs past the made date must equal the one built from the same history stopped at it, at the level of baseline_draws, of the vintage manifest, and of score_release's own wiring. Each pins the centre as well, so a leak moving both ends of the window at once cannot pass by cancelling. The tests pass against the current scorer: no future information reaches the baseline today. They fail if a later vintage reaches the centre, the step pool, or the manifest freeze. Co-authored-by: Sam Abbott <contact@samabbott.co.uk>
The persistence baseline centres on the horizon-length window ending at the made date, or on the last occupancy for the level stream. When that window opens before the vintage stream's first observation, cum_at reads the opening as a zero and the centre saturates at the whole cumulative total to the made date, identical at every horizon. When the vintage manifest carries no dated history for the stream at all, as the earliest release snapshots do, the centre is a zero standing for an absent series and the step pool is empty, leaving a point mass at zero whose CRPS is the observed value. Apply the rule truth_at already applies to the truth window to the baseline's own window (baseline_window_covered) and score no baseline where it is not covered, counted in .no_baseline and logged per release. The group's fits keep their own scores and lose only their relative skill, which was an artefact of the degenerate baseline. The assembled exports stream stays exempt, as it is in stream_coverage_start. Issue #520. Co-authored-by: Sam Abbott <contact@samabbott.co.uk>
![seabbs-review-bot[bot]](https://avatars.githubusercontent.com/in/4624046?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds baseline_window_covered to withhold the persistence baseline (but still score fits) when its centring window opens before a stream's own first vintage, and covers it with substantial new tests in test_baseline_draws.jl and test_scoring.jl. The logic is correct and consistently mirrors the boundary already used by stream_coverage_start/truth_at, and the downstream rel_to_baseline_columns already tolerates a missing baseline row per group; the main issue is that the same rationale is explained in near-identical prose four separate times across the file and the docs example.
Automated first pass by seabbs-review-bot (Claude sonnet), triggered by: first pass. Not a human review. Comment @seabbs-review-bot to ask for another pass: @seabbs any time, the author's agent once it has pushed changes. Add the no-review label to opt this PR out. Ping @seabbs with any questions.
seabbs-review-bot
Bot
added
the
llm-reviewed
Merge note: this PR conflicts with #587Verified with Resolution
The trap. The This was opened by a bot. Please ping @seabbs for any questions. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #586 +/- ##
=======================================
Coverage 93.98% 93.98%
=======================================
Files 15 15
Lines 3493 3493
=======================================
Hits 3283 3283
Misses 210 210 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
📖 Documentation preview is ready! View the docs for this PR at: http://epiforecasts.io/BVDOutbreakSize/previews/PR586/ This preview will be updated automatically when you push new commits. |
2 participants
This is entirely from an agent so do not review until I have pinged for review as I will do a first pass
Why
The maintainer asked for proof that the persistence baseline only ever knows about the past when it is calculated, i.e. rolling with no future bleed.
The baseline is the reference every relative-skill number in the report is measured against, so a baseline that sees the future is unfairly strong and one that degenerates is unfairly weak.
No future bleed, re-derived rather than taken on trust
Issue #520's audit claimed no horizon leakage.
That claim holds, and it is now checked three ways rather than argued.
Every input to
baseline_drawswas traced.vintage_observationsloads the release's own archivedobservations.tomlwithcutoff_date = made_date, andload_observationsdrops every dated vintage after the cut-off inhistory,event_daysand the break-day block alike.cum_attakes the last vintage at or before the date,_history_diffsstops the step pool at the made date, andvintage_grid_dateinverts the frozen manifest's own index mapping exactly.The cache is keyed on
(path, made_date), so a second made date off the same snapshot is frozen again rather than served the first one's freeze.The truth every fit is scored against comes from the current manifest, which is correct.
The baseline is recomputed inside the group loop, per made date, on a window ending at that made date, so it is rolling rather than anchored once.
Checked against the real manifest: for each of three made dates, six streams and three horizons, the baseline drawn from the full manifest (which carries up to eleven further weeks of vintages) is bitwise identical to the one drawn from the manifest frozen at the made date, and each frozen history equals the full one restricted to dates at or before the made date.
The residual gaps documented in #520 are confirmed and unchanged, both revision leakage rather than horizon leakage: the frozen archive's snapshot is the release cut-off rather than the frozen made date, and a failed snapshot fetch falls back to the current manifest with a warning.
The test
test/test_baseline_draws.jlgains four test items, and they are the point of this PR.A vintage snapshot whose history runs past the made date must give the identical baseline to one whose history stops at it, asserted at three levels:
baseline_drawsitself, the manifest throughvintage_observations, andscore_release's own wiring.Each also anchors the centre on its known value, so a leak that moved both ends of the window at once cannot pass by cancelling, and a control assertion shows the two histories do differ once the made date moves past them.
They were verified to fail under three separate deliberate leaks:
cum_atreading the latest vintage regardless of date,_history_diffspooling steps past the made date, andvintage_observationsloading the snapshot at its own cut-off instead of the made date.They pass against the scorer as it stands, which is the evidence for the no-leakage claim.
The fix
Defects 1 and 2 in #520 are the same defect and are fixed together.
The baseline's own backward window was not held to the coverage rule
truth_atalready applies to the truth window.When that window opens before the vintage stream's first observation,
cum_atreads the opening as a zero and the centre saturates at the whole cumulative total to the made date, identically at every horizon.When the vintage manifest carries no dated history for the stream at all, as
results-v1.0.0andresults-v1.1.0do, the centre is a zero standing for an absent series and the step pool is empty, so the baseline is a point mass at zero whose CRPS is exactly the observed value.Both flatter the model.
baseline_window_coveredapplies the existing rule to the baseline's own window, and a group it rejects is scored with no baseline row, counted in.no_baselineand logged per release.The group's fits keep their own scores and lose only their relative skill, which was an artefact.
The assembled exports stream stays exempt exactly as in
stream_coverage_start, since a date before its first detection genuinely says no export had been detected.Falling back to the current manifest would be a leak and is not done.
Estimated against the current manifest, this withholds 6 of 46 baseline rows in
data/forecast_scores.csvand 52 of 88 indata/forecast_scores_frozen.csv.Both point-mass rows named in #520 (
results-v1.1.0 (backfill), 18 May, reported cases and suspected deaths) are among them, as are the saturated 20/23/27 May frozen groups whose baseline centre sat at ~61 confirmed cases against truths of 61, 317, 612 and 832 across the four horizons.The committed score tables are regenerated by the docs build, so they are left alone here.
Defect 3 in #520, the
sqrt(window)justification under drift, is not addressed: it is a modelling judgement rather than a leak, and #520 stays open for it.One residual worth a maintainer's eye, not changed here:
carry_break_dayscarries the harmonisation-break declaration from the current manifest back onto each snapshot.The dates are filtered to the made date, so no future break day enters, and both the step and the printed 24h count are published in the report of the day itself.
The judgement that the step was a base integration is annotated a report or two later, so the baseline holds a label a real-time forecaster would not yet have written down.
With one break day declared (22 July) and every frozen made date before it, nothing currently scored is affected.
Testing
test_baseline_draws.jl,test_scoring.jl,test_stream_coverage.jl,test_rel_to_baseline.jlandtest_break_day_correction.jlall pass (239 tests).Two
score_releasefixtures intest_scoring.jlgain an earlier vintage so their baseline lookback is covered.JuliaFormatter passes over the changed files.
This was opened by a bot. Please ping @seabbs for any questions.