Skip to content

Score the forecast, not the sun - #42

Open
cjaron03 wants to merge 1 commit into
mainfrom
fix/backtest-scoring
Open

cjaron03 wants to merge 1 commit into
mainfrom
fix/backtest-scoring

Conversation

@cjaron03

Copy link
Copy Markdown
Owner

The bug

compute_backtest_metrics reported F1 0.867, precision 93%, recall 81%. None of it measured the model.

The prediction loop appended every sampled timestamp to predictions and never read hazard_score or the probability distribution. There was no decision threshold. A forecast then counted as a hit if a flare fell inside model.max_time_hours, which is 168h, not the 24-48h the product claims.

Over the 13 day test period, 13 of 14 seven day windows contained an M-class flare. 13/14 = 0.929. The reported precision was the base rate.

Reproduced with a model that emits a constant:

forecaster horizon precision F1 Brier skill
constant 0.2/bucket 48h 0.929 0.963 -0.249
constant 0.004/bucket 24h 0.000 0.000 -1.733
separates the classes 24h 1.000 1.000 +0.971

The first row is the published number, produced by a model that knows nothing.

Three more in the same function:

  • false_negatives was computed from m["actuals"][0], so additional flares inside one window were counted as misses. That is the 1 miss against 3 false negatives.
  • The Brier score used sum(probs.values()) across all eight buckets, which is ~1.0 by construction. It measured that the distribution normalised, not that it was calibrated.
  • The backtest period was the same 13 days the model trained on.

The fix

  • Explicit --horizon and --threshold. A probability is not a decision; the threshold has to be stated, and so does the "by when".
  • A real confusion matrix. Every forecast lands in exactly one cell.
  • Every score reported beside climatology, what a forecaster gets for ignoring all inputs and repeating the base rate.
  • Headline is the Brier skill score. Zero means the model added nothing.
  • The exit gate reads skill instead of F1, which rises on its own when events are common.
  • --train-start / --train-end make the report warn when the backtest overlaps training.
  • Horizon is clamped to the model's fitted range rather than scored against extrapolation.

Scoring moved to src/models/backtest_scoring.py: plain arithmetic over plain dicts, no database and no modelling stack, so it is testable on its own.

Tests

tests/test_backtest_scoring.py, 14 cases. The load-bearing ones:

  • a confident constant earns precision equal to the base rate and a skill score <= 0
  • F1 alone would have passed it, and always_yes_f1 is >= the model's
  • false negatives count windows, not flares (the actuals[0] regression)
  • a model that separates the classes scores real skill

Also

The withdrawn figures were displayed in Dashboard.svelte, Predictions.svelte, src/ui/dashboard.py, src/ui/tabs/predictions.py, src/ui/utils/helpers.py and DEPLOYMENT.md. All now say the model is not yet validated, and why.

No performance claim is made anywhere until a backtest runs against a period disjoint from training.

Not verified locally

ui-frontend has no node_modules here, so the Svelte edits are checked by div balance and review rather than a build. CI will build them.

🤖 Generated with Claude Code

The backtest reported F1 0.867, precision 93% and recall 81%. None of
those measured the model.

compute_backtest_metrics counted every sampled timestamp as a positive
forecast, whatever probability the model had emitted. There was no
decision threshold anywhere in the loop. It then counted a forecast
correct if a flare fell inside the model's full 168h range rather than
inside a stated horizon. Over the 13 day test period, 13 of 14 seven day
windows contained an M-class flare, so 13/14 came back as "precision
0.929". Replacing the model with a constant would have scored the same.

Three smaller faults in the same function: false negatives were counted
from actuals[0] of each window, so extra flares in one window were
recorded as misses; the Brier score summed the probability distribution
across all eight buckets, which is ~1.0 by construction, so it measured
that the distribution normalised rather than that it was calibrated; and
the test period was the same 13 days the model trained on.

The scoring now takes an explicit horizon and an explicit threshold,
builds a real confusion matrix, and reports everything beside
climatology. The headline is the Brier skill score, the fraction of
climatology's error the model removed, and the exit gate reads that
instead of F1, which rises on its own when events are common.

Scoring moved to src/models/backtest_scoring.py. It is plain arithmetic
over plain dicts, so it needs neither the database nor the modelling
stack and can be tested directly. tests/test_backtest_scoring.py pins
the regression: a constant model earns precision equal to the base rate
and a skill score at or below zero.

The withdrawn figures were also displayed in the Svelte dashboard, the
Gradio tabs and DEPLOYMENT.md. Those now say the model is not yet
validated and why, rather than repeating a number that described the
sun.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-17T06:01:15.399970Z 16798a5 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16798a5c05

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

BUCKET_LABEL = re.compile(r"^(\d+)h-(\d+)h$")


def reconstruct_survival_pipeline(pipeline_data: Dict) -> SurvivalAnalysisPipeline:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Complete the backtest-helper relocation

The helper move leaves the backtest unusable: this annotation evaluates SurvivalAnalysisPipeline during module import even though it is not imported, and the same file also references undefined Path, datetime, joblib, logger, and FlareEvent. Moreover, scripts/backtest_historical.py imports only compute_backtest_metrics while still calling the moved load_survival_model and get_historical_flares functions. Consequently the scoring tests and backtest cannot start; either keep the database/model helpers in the script or supply all destination dependencies and import the helpers back into the script.

Useful? React with 👍 / 👎.

Comment on lines +112 to +113
if "error" in metrics:
return f"backtest produced no usable result: {metrics['error']}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist the error report when no predictions are produced

When feature generation or prediction fails for every timestamp, this early return bypasses the output_path write below. A previous successful data/backtest_report.txt is therefore left in place even though the current run failed, so automation or users reading the configured output can mistake stale metrics for the latest result. Write or replace the error report before returning.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant