docs(survey): waive zero-weight-PSU SE-invariance item; lock Lumley full-design convention#589
Conversation
…ull-design convention Re-examined the TODO row proposing the survey TSL finite-sample correction count only positive-weight PSUs so the SE is invariant to zero-weight (subpopulation / padded) rows. Investigation shows the premise conflicts with the library's documented, R-validated convention: - `_compute_stratified_psu_meat`'s per-stratum correction (1 - f_h)*n_PSU_h/(n_PSU_h - 1) and PSU-mean centering intentionally keep genuine-subpopulation zero-weight PSUs. This is the full-design domain estimator of Lumley (2004 Section 3.4) / R survey::svyrecvar(subset()), already documented in REGISTRY section "Subpopulation Analysis". - The ATT is exactly invariant; the survey SE is deliberately NOT invariant to genuine-subpopulation zeroing (it should differ from a naive physical subset -- that is the whole point of subpopulation()). R produces the matching SE (only df differs). - Zero-weight rows that reuse an existing PSU label are already bit-invariant. The only invariance-violating shape -- appending synthetic new all-zero PSUs -- arises in no estimator path (domain padding goes through prep.py's zero-padded full-design cell variance, which retains the real PSU layout). Forcing the meat to positive-weight-only counting would break the documented Lumley/R parity, so the item is waived (no estimator behavior change): - TODO.md: move the row from Actionable Backlog to "Won't-fix / waived (decisions on the record)" with the Lumley/R justification. - REGISTRY.md: add a Note in section "Subpopulation Analysis" making explicit that the TSL meat finite-sample correction counts zero-weight PSUs by design. - tests/test_survey.py: add TestZeroWeightPsuConventionWaiver regression-lock (inert existing-PSU padding is bit-invariant; subpopulation zeroing keeps the full PSU structure so its SE differs from a naive subset). A future positive-weight-only change would collapse the two and trip the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t test Addresses the CI review's actionable P3 (Documentation/Tests): the SE-level test only asserts that subpopulation zeroing differs from a physical subset, which catches a full positive-weight-only rewrite but could miss a partial edit (e.g. changing only the finite-sample denominator while still centering over the zero PSU). Adds a direct unit test on _compute_stratified_psu_meat with crafted PSU scores including one all-zero-score PSU (a fully zeroed subpopulation PSU), asserting the exact full-design meat formula (n_PSU_h including the zero PSU) and that it is NOT the positive-weight-only meat. Any change to the centering OR the denominator now trips the lock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment ✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
Methodology Finding: P3 informational, no action required Code Quality No findings. Performance No findings. The PR does not change runtime code. Maintainability No findings. Moving the former backlog item into the waived-decision table makes the convention easier to preserve. Tech Debt No findings. The former action item is now explicitly tracked as waived in Security No findings. Changed files are docs/tests only; no secrets or unsafe execution paths observed. Documentation/Tests Finding: P3 informational |
Addresses the re-review's actionable P3: the direct meat test represented its all-zero PSU via zero score rows only, with weights left all ones. A future denominator-only edit that reads resolved.weights to drop positive-weight PSUs would not have been caught. Set PSU 2's weights to 0 so the fixture models a true fully zero-weight subpopulation PSU. The current meat ignores weights (it operates on scores), so the expected value is unchanged; the change only hardens the regression lock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
_compute_stratified_psu_meat's per-stratum correction(1 - f_h)*n_PSU_h/(n_PSU_h - 1)and PSU-mean centering intentionally keep genuine-subpopulation zero-weight PSUs — the full-design domain estimator of Lumley (2004 §3.4) / Rsurvey::svyrecvar(subset()). The ATT is exactly invariant; the survey SE is deliberately not invariant to genuine-subpopulation zeroing (it should differ from a naive physical subset — the whole point ofsubpopulation()). R produces the matching SE (onlydfdiffers).prep.py's zero-padded full-design cell variance, which retains the real PSU layout; zero-weight rows reusing an existing PSU label are already bit-invariant). Forcing positive-weight-only counting would break the documented Lumley/R parity.TODO.md: moved the row from Actionable Backlog → "Won't-fix / waived (decisions on the record)" with the Lumley/R justification.docs/methodology/REGISTRY.md: added a Note in § "Subpopulation Analysis" making explicit that the TSL meat finite-sample correction counts zero-weight PSUs by design.tests/test_survey.py: addedTestZeroWeightPsuConventionWaiverto lock the decision (inert existing-PSU padding is bit-invariant; subpopulation zeroing keeps the full PSU structure so its SE differs from a naive subset). A future positive-weight-only change would collapse the two and trip the test.Methodology references
survey::svyrecvar(subset()). Existing documentation: REGISTRY § "Subpopulation Analysis".Validation
tests/test_survey.py::TestZeroWeightPsuConventionWaiver(2 tests). Targeted survey zero-weight / subpopulation / invariance subset: 20 passed.Security / privacy