docs(residuals): retire "hybrid" — R2 with any singleton cell is Bishop's ungrouped MA2 - #123
Merged
Merged
Conversation
…op's ungrouped MA2 ## Summary - **What:** the docs, the detector's method token, the analysis-plan text, two docstrings and two test names all said R2 for design state 3 was a per-cell hybrid (exact where n >= 2, MA2 where n = 1). They now say what the code does. - **Why:** the hybrid was removed in cb037f8 (2026-03-05). With any singleton cell, R2 is the 2-point moving average over the full canonical sequence for every observation (Bishop Eq 13.7-13.9), and the 89 ADS 3 reference assertions validate that. The docs described a calculation whose results would fail the reference, three different ways. - **Scope:** sds_detector.py (R2Method, get_r2_method, characteristics, plans), residual_calculator.py and analysis_dataset.py docstrings, synthetic.py docstrings, six docs pages, two test files, CHANGELOG. ## Contract / Invariants (must remain true) - No arithmetic changed: calculate_r2 body untouched; validation/e2e_bishop_report.py exit 0 and docs/reference/validation.md unchanged. - Chart validity and recommendations per design state unchanged. - Vocabulary: R2Method is Literal['exact', 'ma2']; 'within_cell', 'moving_average' and 'hybrid' no longer appear as method values. None of these names is exported from the package top level (CONTRIBUTING: not a public break). ## Behavior Changes (explicit) - get_r2_method returns 'ma2' for mixed cell sizes (was 'hybrid'). - get_sds_characteristics(...)['r2_method'] is 'exact' or 'ma2' for every state (was 'within_cell' / 'moving_average' / 'hybrid' / 'exact' / 'ma2'). - SDSAnalysisPlan.residual_calculation_method uses 'ma2' (was 'moving_average' / 'hybrid'); the state 3 limitation text says what MA2 over the full sequence means. ## Tests - TestR2MethodSelection (new): exact for all-replicated, ma2 for all-singleton and for mixed, and one spelling across plans and characteristics. - Renamed test_calculate_r2_hybrid -> test_calculate_r2_mixed_cells_uses_ma2_for_all and the SDS 3 orchestration test; assertions unchanged in substance. - Three assertions updated from the retired spellings. ## Manual Verification - pytest tests/: 2344 passed, 10 skipped - ruff check .: clean; mypy: 17 pre-existing errors, identical count on main - validation/e2e_bishop_report.py: exit 0 - grep -rni hybrid processbehavior docs tests: only the two comments that explain the removal
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.
Summary
sds_detector.py; docstrings inresidual_calculator.py,analysis_dataset.py,datasets/synthetic.py; six docs pages; two test files; CHANGELOG.Contract / invariants
calculate_r2's body is identical;validation/e2e_bishop_report.pyexits 0 anddocs/reference/validation.mdis unchanged.R2Methodnarrows toLiteral['exact', 'ma2'];get_r2_methodreturns'ma2'where it returned'hybrid';get_sds_characteristics(...)['r2_method']andSDSAnalysisPlan.residual_calculation_methoduse the same two spellings in place of'within_cell','moving_average','hybrid'. None of these names is exported from the package top level, so per CONTRIBUTING this is not a public break; it is recorded under Changed in the CHANGELOG.Behaviour changes
Methodology
docs/user-guide/sds-detection.md("Why DS 3 does not mix methods") asserts the reason MA2 applies to every observation: Bishop's Eq 13.7–13.9 are defined over the whole ordered sequence with no grouping by cell. The equation numbers are from cb037f8's commit message. Please read that paragraph before merge.Tests
TestR2MethodSelection(new,tests/test_sds_detector.py) — exact for all-replicated,ma2for all-singleton and for mixed cells, and one spelling across the analysis plans and characteristics for states 1 to 6.test_calculate_r2_mixed_cells_uses_ma2_for_allandtest_calculate_vas_residuals_sds3_uses_ma2_for_all— renamed; assertions unchanged in substance.Three assertions updated from the retired spellings.
pytest tests/— 2344 passed, 10 skippedruff check .— cleanmypy processbehavior— 17 pre-existing errors, identical count on mainGolden masters untouched
Notes
grep -rni hybrid processbehavior docs testsnow returns only the two comments that explain the removal.r2_methodorresidual_calculation_methodtext; check it for the retired spellings when its library pin moves, not here.🤖 Generated with Claude Code
https://claude.ai/code/session_01BKphCi1LC1hjGYteWEK9JV