Fix propagation scoring and calibration audit #16
No reviewers
Labels
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
graham/prop!16
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/audit-backlog"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What changed
Production evidence
Measurements covered 74,590 valid contacts, 89,331,618 HRRR profiles, 9,220,896 PSK Reporter samples, and 40,081 beacon samples. They support lower-pressure favorability, beneficial 24 GHz moisture during duct formation with path loss modeled separately, and removal of the prior regional multiplier distortion.
The leakage-free recalibration found no independently held-out pre-cutoff negative history sufficient for a fresh per-band fit. All 12 existing overrides are therefore carried forward with
insufficient_temporal_holdoutprovenance rather than publishing a falsely validated change. The canonical validation artifact evaluates 450,365 rows through the real production scorer.Verification
env -u CI make precommit: 4,915 passed, 6 skipped, 22 locally excluded; strict TypeScript, warnings-as-errors compile, unused-dependency check, Credo (6,919 mods/funs, no issues), and xref all passed.cargo clippy --all-targets -- -D warningscargo test --release: 264 passed, 1 ignored.python3 -m py_compile scripts/recalibrate.py scripts/validate_algo.py scripts/validate_forecast.pymix algo.scoreexercised the production JSONL scoring path./aboutand the full/algopage with the new calibration/governance content.Commit:
50a3fe07Review round 2 (
28be681c)All five review findings addressed:
sample/6stores raw conditions;fit_band/2scores train and validation under both candidate and current weights viaScorer.cell_score/3, matchingPskr.Recalibrator.validate_band/5. Report lines now showcandidate delta=… vs current …, auc=… vs ….run_chain_step_hrdpsfetches the preceding step's PRMSL (f-1 hourly, f-3 for the 3-hourly tail) viaHrdpsClient::fetch_variableand attachesprevious_pressure_mb, so Canadian cells take the same tendency branch as HRRR cells.scorer.exandscorer.rs; golden fixture regenerated; new monotonicity property + Rust unit test.superseded(notrejected) with a newapplied_atcolumn; loaders order byapplied_at DESC NULLS LAST, id DESC; the admin page allows Apply on superseded runs for rollback;apply_run/1rejectsrejected/appliedstatuses with{:error, :invalid_status}./algonow showssummary.scored_rows(450,365 held-out rows) instead of the 11 contact rows.Verification:
env -u CI make precommitgreen (4,915 tests),cargo clippy --all-targets -- -D warningsclean,cargo test --release265 passed,scorer_goldenparity holds.🤖 Skippy PR review
5 findings — 2 blocking before merge.
lib/microwaveprop/propagation/recalibrator.ex:220rust/prop_grid_rs/src/pipeline.rs:354lib/microwaveprop/propagation/scorer.ex:579lib/microwaveprop/propagation/algo_weight_runs.ex:62lib/microwaveprop/propagation/validation_report.ex:73First review (242 files, +214588/-7697). Reviewed the scoring/calibration/governance surface: scorer.ex + scorer.rs, band_config on both sides, band_weights.ex/weight_overrides.rs, propagation.ex, path_compute.ex, backtest.ex, backtest/features removal, both recalibrators, validation_report.ex, the algo_weight_runs migration/admin path, hrrr/hrdps clients, pipeline.rs, planes.rs, hrrr_points.rs, adif/csv import, and the 4 migrations. Skipped as low risk: vendored TypeScript/Leaflet types, the 4MB of dated validation artifacts, docs/algo-reports prose, and test-only edits.
Reviewed
50a3fe07dffb. Commentskippy reviewto re-run.@ -489,1 +218,3 @@initial_loss: initial_loss |> Nx.to_number() |> to_float()time: time,label: label,score: Scorer.cell_score(conditions, config),Fixed in
28be681c.sample/6now stores the rawconditionsmap instead of a pre-baked score, andfit_band/2scores both the train and validation rows twice — once under the candidateweights/multipliers, once under the live weights — viaScorer.cell_score/3, the same approach asPskr.Recalibrator.validate_band/5. The report line now readscandidate delta=… vs current …, auc=… vs …so the admin sees the candidates own separation rather than a constant.Fixed in
28be681c.run_chain_step_hrdpsnow fetches the preceding steps PRMSL file alongside the main blob (f-1 for hourly steps, f-3 for the 3-hourly tail, mirroringadd_previous_hrrr_pressures convention) via a newHrdpsClient::fetch_variable, decodes it in the samespawn_blockingclosure, and attachesprevious_pressure_mb— so Canadian cells take the same pressure-tendency branch as HRRR cells. Decode/fetch failures log a warning and fall back to the absolute branch.algo.mddocuments the behavior.Fixed in
28be681c. The delta branch is now monotone: deltas <= -2.5, -0.8, +0.5, +2.0 mb score 80, 70, 60, 55, and larger rises score 45 — matching the documented direction (falling = improving). Same table inscorer.rs;priv/rust_golden/scores.bincoderegenerated andscorer_goldenpasses. Added ascore_pressureproperty asserting the delta branch is non-increasing in delta, plus a Rust unit test over the same deltas.Fixed in
28be681c. Retired runs are now markedsuperseded(distinct fromrejected), and a newapplied_atcolumn records when each run went live — so the table answers "which candidate was live between A and B".latest_applied_payloadand the Rustweight_overridesloader both order byapplied_at DESC NULLS LAST, id DESC, removing the same-secondinserted_atnondeterminism. The admin page renders Apply onsupersededruns (rollback path), andapply_run/1rejectsrejected/appliedstatuses server-side with{:error, :invalid_status}. Migration20260922121035adds the column.Fixed in
28be681c. The stat now readssummary.scored_rows(450,365 in the 2026-09-22 artifact) and the/algolabel says "held-out rows" instead of "held-out QSOs", so the PSKR and beacon mass is no longer hidden behind the 11 contact rows.@ -0,0 +59,4 @@|> Multi.update_all(:retire_previous,from(r in AlgoWeightRun, where: r.status == "applied" and r.id != ^run.id),set: [status: "rejected"]🔵 Suggestion — Superseded run is recorded as rejected, erasing the applied history
apply_run/1retires the previously applied run by settingstatus: "rejected", which is the same value an administrator sees for a candidate nobody applied and the same valuelatest_applied_payload/0filters out. After a second apply, the table can no longer answer "which candidate was live between date A and date B", and because the LiveView only renders Apply forstatus == "pending"there is no way to roll back to the previously live weights. Suggest a distinct state (superseded) or asuperseded_by/retired_atpair, with the page allowing Apply on a superseded run. Note also thatinserted_atis truncated to the second, so two runs inserted in the same second makeORDER BY inserted_at DESC LIMIT 1nondeterministic for both the Elixir loader andweight_overrides.rs.@ -489,1 +218,3 @@initial_loss: initial_loss |> Nx.to_number() |> to_float()time: time,label: label,score: Scorer.cell_score(conditions, config),🟡 Warning — Held-out validation scores the live weights, not the candidate
sample/6storesScorer.cell_score(conditions, config), so every train and validation row is scored with the weights resolved today.fit_band/2then attachesvalidation_summary(split.validation)to the result, and that text is what an administrator reads in thereportcolumn of/admin/algo-weights(score_delta=...). The candidate's ownweights/multipliersare never applied to the holdout, so the reported separation describes the current scorer and cannot show whether the candidate is better or worse: the same number comes out for every candidate fit on the same window.Scorer.cell_score/3exists for exactly this and the sibling fitter in this PR (Pskr.Recalibrator.validate_band/5) uses it, scoring the holdout twice, with candidate weights and withnil. Score the validation rows both ways here too, and report candidate vs current AUC/score delta in the report string.@ -578,0 +576,4 @@defp score_pressure_delta(delta) when delta < -2.5, do: 80defp score_pressure_delta(delta) when delta < -0.8, do: 70defp score_pressure_delta(delta) when delta < 0.5, do: 60defp score_pressure_delta(delta) when delta < 2.0, do: 65🔵 Suggestion — Pressure-trend curve is not monotone: slowly rising beats steady
The clauses are ordered 80 (
delta < -2.5), 70 (< -0.8), 60 (< 0.5), 65 (< 2.0), 45. A pressure rising 0.5-2.0 mb returns 65 while a nearly steady one (-0.8 to 0.5) returns 60, so a small rise scores better than flat, opposite to this PR's documented direction ("falling pressure is improving and rising pressure is degrading"). The pre-inversion table carried the same artefact in the mirror band (delta > -2.0-> 65 next todelta > -0.5-> 60), so this looks like the old ordering preserved through the sign flip rather than a deliberate shape. Make the thresholds monotone in delta (the 65 band should sit below the 60 band) in bothlib/microwaveprop/propagation/scorer.exandrust/prop_grid_rs/src/scorer.rs, and regeneratepriv/rust_golden/scores.bincode` in the same commit.@ -54,2 +72,2 @@bands: length(@eligible_gains),test_contacts: get_in(@report, ["summary", "test_contacts"]),bands: length(@tested_values),test_contacts: get_in(@report, ["summary", "contact_rows"]) || 0,🔵 Suggestion — Validation stat understates the held-out corpus by four orders of magnitude
test_contactsnow readssummary.contact_rows, which is 11 indocs/algo-reports/validation-2026-09-22.json(only three-position contacts) whilesummary.scored_rowsis 450,365./algorenders it as "median rho over 7 bands - 11 held-out QSOs", so the public page tells readers the score validation rests on 11 QSOs when the artifact scored 450,365 held-out rows (450,116 PSKR, 238 beacon, 11 contact). Point the stat atsummary.scored_rows, or split it per source so the PSKR and beacon mass is visible.@ -350,3 +352,3 @@let fused = tokio::task::spawn_blocking(move || {derive_and_score(&grid, valid_time, false, Some(&mask), kp)derive_and_score(&grid, valid_time, false, Some(&mask), kp, &weights)🟡 Warning — HRDPS steps never get previous pressure: pressure factor means two different things
add_previous_hrrr_pressureis called only fromrun_chain_step(line 161) andrun_analysis_step(line 1587). So every HRDPS forecast step keepsprev_pressure_mb: Noneand lands inscore_pressure/2's absolute branch (30-88, keyed on absolute MSL pressure), while HRRR-owned cells take the delta branch (45-80, keyed on a 1-hour tendency that is usually near zero). Pressure is one of the ten weighted factors, so an HRRR-owned cell and an HRDPS-owned cell on the border are scored on different quantities for the same valid time, which contradicts the PR's stated HRRR/HRDPS input consistency. Either fetch the preceding HRDPS step's PRMSL inrun_chain_step_hrdpsor make both models use the absolute branch deliberately; right now which branch runs is an artifact of which model owns the cell.Resolved all 5 earlier findings in
28be681c:nil), so the report'scandidate delta/aucvscurrentfinally describes the candidate.previous_pressure_mb, so Canadian cells take the same tendency branch as HRRR cells.algo.mdregenerated with it.superseded+applied_atpreserve the applied history, and a superseded run is re-appliable as the rollback path (latest_applied_payloadandweight_overrides.rsboth order by apply time, not insert time)./algoreports held-out rows (450,365) instead of 11 QSOs.Nothing new in
50a3fe07..28be681c. Check is green.