efficient-did: thread vcov_type as narrow {hc1} contract per Chen-Sant'Anna-Xie 2025 (Phase 1b #4)#495
Merged
Merged
Conversation
…t'Anna-Xie 2025 (Phase 1b interstitial #4) Mirrors the IF-based narrow-contract template from CallawaySantAnna #487, TripleDifference #488, and ImputationDiD #492. EfficientDiD uses influence-function-based variance per Chen-Sant'Anna-Xie (2025) achieving the semiparametric efficiency bound; the per-unit EIF aggregation has no equivalent single design matrix, so analytical-sandwich families cannot be defined and vcov_type is permanently narrow to {"hc1"}. Input contract: - vcov_type kwarg threaded through __init__ / get_params / set_params - _validate_vcov_type rejects {classical, hc2, hc2_bm} with methodology-rooted message citing Chen-Sant'Anna-Xie (2025) and the missing design matrix - conley rejected as deferred (TODO.md follow-up row) - set_params(vcov_type=bad) raises immediately via _validate_params chain (intentional eager-validation; diverges from sibling IF-based estimators which defer to fit-time per sklearn mutate-then-validate-at-use) EfficientDiDResults harmonization (BC): - cluster field renamed to cluster_name (matches IF-based estimator naming) - New n_clusters field for the G=<n> suffix on the variance summary line - New vcov_type field - New to_dict() method (mirrors TripleDifferenceResults / ImputationDiDResults) - summary() now renders "Variance estimator: <label>" after the survey block, suppressed under bootstrap (renders "Inference method: bootstrap" instead) - Default cluster=None renders "HC1 heteroskedasticity-robust" — methodologically correct (per-unit EIF SE is HC1-style, no Liang-Zeger G/(G-1) correction); diverges from ImputationDiD which auto-clusters at unit per BJS Theorem 3 - DiagnosticReport._pt_hausman updated to read renamed cluster_name field Bootstrap defensive fix: - Survey-PSU multiplier bootstrap with G<2 PSUs collapses to constant draws → BLAS roundoff produces ≈0 SE (not NaN). Single-site guard at the survey-PSU branch returns NaN dicts keyed to the same (g,t)/event-time/ group reductions the downstream override loop expects (cluster path already guards n_clusters≥2 at fit-time; unit path guarded upstream by balanced-panel validator). - New _build_nan_bootstrap_results helper constructs the all-NaN object. Tests (40 new in TestEfficientDiDVcovType across 7 surfaces): - Default / cluster / TSL-survey / replicate-survey bit-equal parametrized over aggregate ∈ {None, event_study, group, all} - Bootstrap × cluster + bootstrap × survey bit-equal (per-horizon + per-group SE override branches asserted) - set_params eager revalidation - Bootstrap n_psu<2 NaN propagation (regression on defensive fix) - DR (covariates=) path bit-equal under explicit vcov_type="hc1" - 5 input-rejection pins (classical/hc2/hc2_bm/conley/unknown) - cluster + replicate_weights rejection (validates blanket guard at L357) - 7 introspection tests (default attr, get_params, Results carries, to_dict default vs cluster, summary HC1 default vs CR1 cluster vs bootstrap-suppressed, cluster_name suppression under both analytical and replicate survey, fit-clone idempotence) Documentation: - REGISTRY.md: IF-based taxonomy "Enforced today" expanded to four-way (CS+TD+ImputationDiD+EfficientDiD); 3 EfficientDiD Notes added - llms-full.txt: vcov_type added to EfficientDiD signature; shared staggered-results variance metadata section updated - CHANGELOG [Unreleased] entry surfaces both vcov_type threading AND the Results-field BC break - TODO.md Phase 1b row collapses to TwoStageDiD only (last remaining); new EfficientDiD Conley follow-up row added - EfficientDiD class + module + Results docstrings updated 210 tests pass across test_efficient_did.py + test_efficient_did_validation.py + test_diagnostics.py. black + ruff clean.
…plicate Bootstrap summary line
Two follow-up fixes:
1. cluster → cluster_name propagation. The original PR renamed the
EfficientDiDResults.cluster field to cluster_name but left two
consumers still referencing the old name:
- tests/test_business_report.py:1267 (TestHausmanPretestPropagatesCluster
regression test asserting the field is persisted for the Hausman
pretest replay) — updated assertion + class docstring.
- docs/api/_autosummary/diff_diff.EfficientDiDResults.rst — the
checked-in Sphinx autosummary still listed cluster and was missing
cluster_name, n_clusters, vcov_type, and the new to_dict() method.
2. Suppress duplicate Bootstrap summary header. EfficientDiDResults.summary()
previously emitted both the legacy "Bootstrap: <n> (<weights>)" header
line (L250-L251 pre-PR) and the new "Inference method: bootstrap" +
"Bootstrap replications: <n>" block. The legacy header is now gated on
`bootstrap_results is None` so analytical-only fits keep the original
render and bootstrap-overwritten fits use the canonical inference-method
block alone (matches the sibling result-container convention).
211 tests pass across test_efficient_did.py, test_efficient_did_validation.py,
test_diagnostics.py, and test_business_report.py::TestHausmanPretestPropagatesCluster.
black + ruff clean on touched Python files.
|
Overall Assessment ✅ Looks good Executive Summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
|
…ons on validation failure Previously, EfficientDiD.set_params iterated over the kwargs and called setattr() before invoking _validate_params(). A rejected batched call such as set_params(vcov_type="classical", alpha=0.1, anticipation=2) raised on validation but left every kwarg in the partially-mutated state — the rejected vcov_type plus the otherwise-valid alpha and anticipation. Callers that catch ValueError and continue using the estimator would then operate on a silently corrupted parameter configuration, defeating the eager-validation contract introduced when _validate_vcov_type was wired into _validate_params. Fix: snapshot original attribute values for every kwarg before applying mutations, run validation, and on exception restore the snapshot before re-raising. The snapshot pass also moves the "Unknown parameter" check ahead of any mutation so that even unknown-name rejections leave the estimator untouched. Regression test added: test_set_params_rollback_on_validation_failure fires the 3-kwarg batched call and pins all three attribute values to their pre-call snapshot after the raise.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment ✅ Looks good Executive Summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
|
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
1 participant
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
vcov_typethroughEfficientDiDas permanently narrow to{"hc1"}per Chen-Sant'Anna-Xie (2025) EIF-based variance achieving the semiparametric efficiency bound. Analytical-sandwich families{classical, hc2, hc2_bm}andconleyare rejected at__init__/set_paramswith methodology-rooted messages (the per-unit EIF aggregation has no equivalent single design matrix on which hat-matrix leverage or Bell-McCaffrey Satterthwaite DOF can be defined).EfficientDiDResults:clusterfield renamed tocluster_name; newn_clusters+vcov_typefields added; newto_dict()method (mirrorsTripleDifferenceResults/ImputationDiDResults).DiagnosticReport._pt_hausmanupdated to read the renamed field.Variance estimator: <HC1 / CR1 cluster-robust at <col>, G=<n>>after the survey block; suppressed under bootstrap (replaced withInference method: bootstrap+Bootstrap replications: <n>). Defaultcluster=Nonerenders "HC1 heteroskedasticity-robust" — methodologically correct (per-unit EIF SE is HC1-style with no Liang-Zeger G/(G-1) correction); diverges fromImputationDiDwhich auto-clusters at unit per BJS Theorem 3.<2 PSUsnow returns NaN SE (was ≈0 from BLAS roundoff). Single-site guard in the survey-PSU branch + new_build_nan_bootstrap_resultshelper that keys NaN dicts to the same (g,t) / event-time / group reductions the downstream override loop expects.EfficientDiD.set_params(vcov_type=bad)raises immediately (matches existing pattern forpt_assumption/control_group/etc); intentional deviation from sibling IF-based estimators which use sklearn mutate-then-validate-at-use. Documented in REGISTRY.md.Methodology references (required if estimator / math changes)
vcov_type={"hc1"}contract — structural property of the IF-based variance derivation (no global design matrix on which HC2 leverage or Bell-McCaffrey DOF can be defined). Documented indocs/methodology/REGISTRY.md"IF-based variance estimators vs analytical-sandwich estimators" section + EfficientDiD per-estimator Notes. Matches the established pattern acrossCallawaySantAnna/TripleDifference/ImputationDiD.cluster=Nonerenders HC1, NOT CR1-at-unit — per-unit EIF SEsqrt(mean(EIF²)/n)is HC1-style (no Liang-Zeger G/(G-1) finite-sample correction). Diverges fromImputationDiDwhich auto-clusters at unit per BJS Theorem 3.set_params(vcov_type)validation —EfficientDiD.set_paramscalls_validate_params()which now invokes_validate_vcov_type, so bad vcov raises immediately rather than at fit-time. Matches EfficientDiD's existing eager-validation pattern; diverges from sibling IF-based estimators (sklearn mutate-then-validate-at-use). Documented in REGISTRY.md.Validation
tests/test_efficient_did.py— newTestEfficientDiDVcovTypeclass with 40 tests across 7 surfaces (default / cluster / TSL-survey / replicate-survey bit-equal parametrized overaggregate ∈ {None, event_study, group, all}; bootstrap × cluster + bootstrap × survey bit-equal with per-horizon/per-group SE override branches asserted;set_paramseager revalidation; bootstrap n_psu<2 NaN propagation; DR (covariates=) path bit-equality; 5 input-rejection pins;cluster + replicate_weightsrejection via blanket guard; 7 introspection tests).tests/test_business_report.py::TestHausmanPretestPropagatesCluster— assertion updated fromgetattr(edid, "cluster", None)→getattr(edid, "cluster_name", None)to track the field rename.vcov_type="hc1"is bit-equal to current default across all aggregate modes, the DR covariates path, and all bootstrap branches; no R parity needed.Security / privacy