fix(survey): unify explicit-vcov_type contract under replicate designs (warn + hc1 remap)#654
fix(survey): unify explicit-vcov_type contract under replicate designs (warn + hc1 remap)#654igerber wants to merge 1 commit into
Conversation
Overall AssessmentExecutive Summary
MethodologyFinding 1 — P1Location: Impact: The new helper says explicit non- Concrete fix: Compute an effective vcov family before any vcov-dependent routing, e.g. Code QualityFinding 2 — P3Location: Impact: The replicate-vcov warning can fire before the Concrete fix: Move the PerformanceNo separate performance blocker. The P1 also has a performance side effect: absorbed DiD/MPD replicate fits may still build full-dummy designs under an effective MaintainabilityNo additional maintainability findings beyond the P1 routing split between Tech DebtNo untracked blocking tech debt found. Removing the old TODO row is acceptable if the implementation and registry are made internally consistent. SecurityNo security issues found in the changed files. Documentation/TestsFinding 3 — P2Location: Impact: The registry now contradicts itself. The new replicate-variance note says TWFE Concrete fix: Update the older TWFE paragraph to reference the new replicate-weight note and remove the stale Finding 4 — P2Location: Impact: Tests cover TWFE Concrete fix: Parametrize the replicate-vcov tests across Path to Approval
|
…s (warn + hc1 remap) With replicate variance the analytical sandwich is replaced wholesale: per-replicate refits return point estimates only, identical across vcov families (FWL: full-dummy and within-transformed fits give the same coefficients), so the requested vcov family cannot influence any reported number. TwoWayFixedEffects(vcov_type='hc2'/'hc2_bm') raised NotImplementedError while DifferenceInDifferences silently ignored the kwarg — inconsistent twins. All three (incl. MultiPeriodDiD) now share one contract via a base-class helper: explicit non-hc1 vcov_type warns and the (discarded) base fit remaps to hc1 — avoiding wasted CR2-BM work, one-way validator rejections, and the TWFE full-dummy auto-route (which does not compose with per-replicate re-demeaning). Explicit hc1 (the old error's own workaround guidance) stays silent. The per- replicate full-dummy HC2 implementation the TODO row proposed was investigated and rejected as a costly no-op: it cannot change the replicate variance. Tests lock warn + bit-identity-to-hc1 on all three estimators; REGISTRY survey-section Note documents the decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fe4446d to
b399219
Compare
Summary
uses_replicate_variancethe analytical sandwich is replaced wholesale — the per-replicate refits return point estimates only, which are identical across vcov families (FWL: the full-dummy and within-transformed fits give the same coefficients) — so the proposed per-replicate full-dummy HC2 refit is a costly no-op that cannot change the replicate variance. Verified empirically before changing anything: DiDhc2+replicate already produced bit-identical output tohc1+replicate, silently.TwoWayFixedEffects(vcov_type="hc2"/"hc2_bm")raisedNotImplementedErrorwhileDifferenceInDifferencessilently ignored the kwarg. All three estimators (incl.MultiPeriodDiD) now share one contract via a base-class helper: explicit non-hc1analyticalvcov_type(hc2/hc2_bm/classical) warns ("has no effect with replicate-weight survey designs…") and the discarded base fit remaps tohc1— avoiding wasted CR2-BM work, one-way-only validator rejections, and the TWFE full-dummy auto-route (which does not compose with per-replicate re-demeaning). Explicithc1— the old error message's own workaround guidance — stays silent.conleyis excluded: it carries its own survey-design support contract (TSL stratified-Conley sandwich; dedicated validators), which keeps firing unchanged (local-review P1, locked by a contract test).Methodology references (required if estimator / math changes)
Validation
tests/test_replicate_weight_expansion.py::TestReplicateVcovTypeWarn(4 tests),tests/test_estimators_vcov_type.py(raise test → warn+identical, both vcov params). Impacted suites green: 522 tests (replicate expansion, vcov-type, estimators, survey, survey_phase6).Security / privacy
Generated with Claude Code