Cleanup: drop stale R-learner refs, strip finding numbers, retire spe… - #134
Merged
Conversation
…nt drivers Post-C7 housekeeping ahead of the campaigns tranche. - Reword all "R-learner" references left behind by the rlearner package deletion (C7) to describe current behaviour. Notably the overnight studies advertised "oracle + naive + R-learner + v0" but now run power_model. - Strip finding-number citations (F1/F5/F13/F30/...) from source and tests per the CLAUDE.md rule; the rationale stays in docs/v1/findings.md. - Move spent one-off / manual inspection drivers into benchmarking/baselines/old/: migrate_toggle_baseline_v2_to_v3, inspect_era5_matching_importance, inspect_short_campaigns, inspect_naive, inspect_v0_run (no tests, not imported by the active tree). poe all-fast green; no scored-method or benchmark behaviour changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Housekeeping PR that updates benchmarking/diagnostic text after the rlearner package deletion, removes “finding number” citations from inline comments/docstrings per CLAUDE.md, and groups retired one-off/manual drivers under benchmarking/baselines/old/.
Changes:
- Reword stale “R-learner” references to describe current
power_model-based behavior. - Remove many inline finding-number citations (Fxx) from source and tests while keeping rationale in docs.
- Create
benchmarking.baselines.oldand update “how to run” module paths for retired drivers.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/benchmarking/baselines/test_study_toggle_methods_compare.py | Removes a finding-number citation from the toggle-methods harness docstring. |
| tests/benchmarking/baselines/test_study_power_model_compare.py | Rewords comments to remove finding-number references around power_model defaults. |
| tests/benchmarking/baselines/test_power_model_method.py | Rewords docstrings/comments to drop finding-number citations (e.g., “shrinkage mechanism”). |
| tests/benchmarking/baselines/test_block_bootstrap.py | Rewords several docstrings/comments to remove finding-number citations. |
| benchmarking/synthetic/schema.py | Updates schema docstring to remove finding-number references. |
| benchmarking/diagnostics/northing.py | Rewords docstring to remove “R-learner” framing. |
| benchmarking/diagnostics/histograms.py | Rewords docstring to generalize from “R-learner” to “models”. |
| benchmarking/diagnostics/context.py | Rewords docstring/comments to remove “R-learner” specific internals. |
| benchmarking/baselines/toggle_specialist.py | Rewords docs/comments to remove finding-number and “R-learner” references. |
| benchmarking/baselines/study_toggle_specialist_uncertainty.py | Removes finding-number citations from design-point commentary. |
| benchmarking/baselines/study_toggle_methods_compare.py | Removes finding-number citations from docstrings/comments and warning text. |
| benchmarking/baselines/study_power_model_compare.py | Removes a finding-number citation from the machine-specific benchmark description. |
| benchmarking/baselines/study_overnight_toggle.py | Updates advertised method list from “R-learner” to power_model. |
| benchmarking/baselines/study_overnight_prepost.py | Updates docs to refer to power_model (and lightgbm dependency) instead of “R-learner”. |
| benchmarking/baselines/power_model/method.py | Rewords extensive module/class docs to drop finding-number citations while keeping rationale. |
| benchmarking/baselines/power_model/matching.py | Rewords docs to remove “R-learner” finding citation. |
| benchmarking/baselines/power_model/features.py | Rewords docs to remove “R-learner” comparison language. |
| benchmarking/baselines/old/migrate_toggle_baseline_v2_to_v3.py | Moves/labels a one-off migration driver under old/ and updates its python -m path. |
| benchmarking/baselines/old/inspect_v0_run.py | Updates python -m invocation to the old module path. |
| benchmarking/baselines/old/inspect_short_campaigns.py | Updates python -m invocation to the old module path. |
| benchmarking/baselines/old/inspect_naive.py | Updates cross-reference and python -m invocation to the old module path. |
| benchmarking/baselines/old/inspect_era5_matching_importance.py | Updates docs and python -m invocation; removes finding-number citations. |
| benchmarking/baselines/old/init.py | Adds package marker/docstring describing retired drivers and how to run them. |
| benchmarking/baselines/naive_ratio.py | Rewords docs to reference power_model instead of “R-learner”. |
| benchmarking/baselines/inspect_prepost_hard_case.py | Rewords docs/comments to remove finding-number citations and “R-learner” framing. |
| benchmarking/baselines/hot_context.py | Rewords docs to remove stale “Issue 5 R-learner” forward-looking reference. |
| benchmarking/baselines/filtering.py | Rewords docs to remove “R-learner” specific references and clarify method behavior. |
| benchmarking/baselines/example_toggle_study.py | Rewords comments describing accepted defaults without finding-number citations. |
| benchmarking/baselines/example_prepost_study.py | Rewords comments describing accepted defaults without finding-number citations. |
| benchmarking/baselines/era5_sync.py | Rewords docs to remove “R-learner” back-compat framing. |
| benchmarking/baselines/block_bootstrap.py | Removes finding-number citation from module docs and rewords parameter/docstrings. |
Suppressed comments (1)
benchmarking/baselines/old/migrate_toggle_baseline_v2_to_v3.py:8
- This module is now under
benchmarking.baselines.oldandbenchmarking/baselines/old/__init__.pystates these scripts are retained for reference, but the docstring still instructs readers to delete it. That’s internally inconsistent and may confuse how long this script is expected to live.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
34
to
+36
| **This benchmark is machine-specific: record and diff it on one machine only.** Every cell here is | ||
| ``power_model``, and LightGBM's threaded float reduction order depends on the machine, so a benchmark | ||
| recorded elsewhere reports a permanent false MOVED of ~0.7 pp — 14x the same-machine noise (F30). | ||
| recorded elsewhere reports a permanent false MOVED of ~0.7 pp — 14x the same-machine noise. |
Comment on lines
229
to
231
| class TestTooFewRecordsToFallBackOn: | ||
| """A cell too sparse to bootstrap must still get an honest, wide sigma — not 0, and not NaN (F33). | ||
| """A cell too sparse to bootstrap must still get an honest, wide sigma — not 0, and not NaN. | ||
|
|
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.
…nt drivers
Post-C7 housekeeping ahead of the campaigns tranche.
poe all-fast green; no scored-method or benchmark behaviour changes.