V1 toggle specialist#126
Conversation
Adds a weeks-based campaign-length grid alongside the existing months grid, and a study script that regression-tests the two toggle-capable methods (toggle_specialist, power_model) on Hill of Towie. The weeks grid is additive: StudyConfig takes exactly one of campaign_months / campaign_weeks, and leaderboard/plot_campaign_curves take a length_col defaulting to campaign_months. Every existing months-based call is therefore bit-identical, so the committed power_model baseline stays valid. months-only accessors (CampaignWindow.months, StudyConfig.max_activity_months) raise on a weeks study rather than silently reporting weeks as months. study_toggle_methods_compare.py scores a placebo plus a symmetric +/-2% Cp pair over 1/2/4/8 weeks -- the small-signal, short-campaign regime a real toggle campaign lives in. It reports raw deltas against a committed benchmark: ground truth is deterministic in (config, seed), so an unchanged method must diff to exactly 0.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new toggle-only baseline method (toggle_specialist) plus a dedicated short-campaign (weeks-grid) regression harness, while generalizing the harness to support campaign-length grids in either months or weeks and standardizing conditional-axis selection/validation across methods.
Changes:
- Introduce
ToggleSpecialistMethod(toggle-only energy-ratio baseline) with optional per-power-bin reporting and rich diagnostics. - Generalize campaign windowing/scoring/plotting/leaderboards to support a weeks-based campaign grid in addition to months.
- Replace
PowerModelMethod.conditional_upliftwithconditions=...+ sharedvalidate_conditions, and add a new toggle-methods benchmark script + committed baseline.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/benchmarking/harness/test_replicates.py | Adds coverage for weeks-grid studies and campaign-grid validation on StudyConfig. |
| tests/benchmarking/harness/test_conditions.py | Adds unit tests for shared validate_conditions. |
| tests/benchmarking/harness/test_campaign.py | Adds weeks-grid campaign-window tests and grid-validation tests. |
| tests/benchmarking/baselines/test_toggle_specialist.py | Comprehensive unit tests for the new ToggleSpecialistMethod behavior and diagnostics. |
| tests/benchmarking/baselines/test_study_toggle_methods_compare.py | Tests the new toggle-methods regression harness behavior (selection, leaderboard, diffing, plots). |
| tests/benchmarking/baselines/test_study_power_model_compare.py | Updates expectations to the new conditions API. |
| tests/benchmarking/baselines/test_power_model_method.py | Migrates tests from conditional_uplift to conditions and adds conditions-selection tests. |
| tests/benchmarking/baselines/test_naive_ratio.py | Minor formatting-only adjustments. |
| docs/v1/findings.md | Documents the new toggle-specialist per-bin estimator and the new weeks-based regression harness. |
| benchmarking/synthetic/ground_truth.py | Clarifies comments re: baseline/upgrade invariance for power binning. |
| benchmarking/harness/scoring.py | Records campaign length under the correct column (campaign_months vs campaign_weeks). |
| benchmarking/harness/replicates.py | Extends StudyConfig to accept exactly one of months/weeks grids; adds generic accessors. |
| benchmarking/harness/plots.py | Generalizes campaign-curve plotting to a configurable campaign-length column. |
| benchmarking/harness/leaderboard.py | Generalizes leaderboard grouping keys via a configurable campaign-length column. |
| benchmarking/harness/conditions.py | Adds shared validate_conditions and updates power-bin comment wording. |
| benchmarking/harness/campaign.py | Adds weeks-grid support, CampaignUnit, generic window length fields, and grid resolution/validation. |
| benchmarking/diagnostics/density.py | Updates module docstring text (comment-only change). |
| benchmarking/baselines/toggle_specialist.py | New toggle-only baseline method implementation + diagnostics and optional per-bin reporting. |
| benchmarking/baselines/study_toggle_methods_compare.py | New weeks-grid regression harness for toggle methods with benchmark record/diff workflow. |
| benchmarking/baselines/study_toggle_methods_compare_baseline.json | Adds committed benchmark baseline cells for the new harness. |
| benchmarking/baselines/study_power_model_compare.py | Updates baseline provenance to use campaign_lengths accessor. |
| benchmarking/baselines/power_model/method.py | Replaces conditional_uplift with conditions, adds validation, and updates conditional power bin labeling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "schema": "toggle_methods_compare_baseline_v2", | ||
| "recorded_utc": "2026-07-15T11:26:05Z", | ||
| "git_commit": "4aa7b5a-dirty", | ||
| "n_replicates": 4, |
There was a problem hiding this comment.
Agreed that a committed benchmark must be reproducible from its commit, and this one will be re-recorded on a clean tree before merge.
Flagging that the suggested fix alone would leave the underlying trap in place, though. _git_commit() reads HEAD at write time, not run start. A full sweep takes ~15 minutes, so a commit landing mid-run mislabels the baseline no matter how clean the checkout was when it started. That is not hypothetical — it is how the first baseline here came to be stamped 04f36d6 while actually running f6b509b's method code. Re-recording from a clean checkout fixes this instance and the next person hits the same thing.
So both are addressed:
- Capture before the sweep. The commit is read at run start and threaded into
record_baselinerather than looked up when the JSON is written. Pinned bytest_baseline_records_the_commit_it_was_given_not_the_current_head. - Refuse rather than warn.
--update-baselinenow errors on a dirty tree instead of writing an untraceable benchmark, and fails immediately rather than after 15 minutes of compute:
error: refusing to --update-baseline from a dirty tree (commit 7ebae0b-dirty): the committed
benchmark must be reproducible from its commit. Commit your changes first, then re-run.
A plain reporting run (no --update-baseline) is unaffected — a dirty tree is fine there, since nothing is committed.
One expected consequence worth recording: because power_model is not bit-reproducible (see the other thread), the re-recorded cells will differ from the current ones by up to ~0.05 pp at campaign_weeks=1. That is its measured noise floor, not a regression.
| logger.info( | ||
| "Toggle methods vs benchmark (recorded %s, commit %s) [pp]; an unchanged method reads d_*=0.0 exactly:\n%s", | ||
| prov.get("recorded_utc", "?"), | ||
| prov.get("git_commit", "?"), | ||
| show.to_string(index=False), | ||
| ) |
There was a problem hiding this comment.
Correct — that line is stale and contradicts the script's own verdict two lines below it.
The "exactly 0.0" wording came from the original design premise, which the first real before/after run disproved: power_model is not bit-reproducible run to run despite its seed (LightGBM's threaded float reduction order varies; seed governs sampling, not that). Measured on two runs of identical code: 5e-4 (0.05 pp) at campaign_weeks=1, and exactly 0.0 at 2 and 8 weeks — the noise is sparsity-driven, since with a week of data the model sits near a split boundary and a tiny float difference flips a tree. toggle_specialist, being pure arithmetic, does reproduce exactly. Hence the per-method bands, plus the baseline's round(8) residual.
The module docstring, _log_unchanged_verdict and the _UNCHANGED_ATOL comment were all updated at that point; this logger.info was missed. It now names each method's actual band instead of claiming exactness:
Toggle methods vs benchmark (recorded %s, commit %s) [pp]; unchanged = within each method's band
(toggle_specialist 1e-05 pp, power_model 0.1 pp), reported per method below:
Add a new toggle specialist model and study focused on short toggle campaigns