Skip to content

feat(v4): n_bootstrap validation sweep + fail-closed inference selector (2(d) PR-B, M-081/M-096) - #754

Open
igerber wants to merge 1 commit into
mainfrom
feat/v4-2d-inference-policies
Open

feat(v4): n_bootstrap validation sweep + fail-closed inference selector (2(d) PR-B, M-081/M-096)#754
igerber wants to merge 1 commit into
mainfrom
feat/v4-2d-inference-policies

Conversation

@igerber

@igerber igerber commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • M-081 (done): promote a shared validate_n_bootstrap to diff_diff/utils.py (verbatim from ChangesInChanges' local validator: non-negative integer, numpy integers accepted, bool/None/float/negative rejected) and apply it at __init__ across the previously-unvalidated roster - CallawaySantAnna, SunAbraham, EfficientDiD, ImputationDiD, TwoStageDiD, WooldridgeDiD, ContinuousDiD, StaggeredTripleDifference, and the DiD family (DiD's __init__; MultiPeriodDiD/TwoWayFixedEffects inherit); CiC/QDiD re-point to the shared helper with a byte-identical message. 0 stays legal and still means bootstrap off on every > 0-gated analytical lane; no numeric defaults change. Named exception recorded in the ledger + design doc: HAD keeps its >= 1 floor (its n_bootstrap powers only the optional sup-t band, whose off-switch is fit(cband=False)).
  • M-096 (done): the inference selector fails closed. Accepted set is exactly {"analytical", "wild_bootstrap"}, string-typed (an isinstance guard closes the one-element-ndarray hole in bare tuple membership), at __init__ and transactional set_params. At fit - placed after the survey and Conley front doors so their NotImplementedError rejections keep precedence - DiD with wild_bootstrap and no cluster= raises ValueError (previously a silent analytical fallback; test_did_wild_bootstrap_requires_cluster flips BY DESIGN, disclosed in CHANGELOG), and DiD/TWFE with n_bootstrap < 2 raise (n_bootstrap ∈ {0, 1} ran WCR with too few draws and returned a wild-labeled all-NaN inference tuple; the < 2 floor amends the originally-locked < 1, recorded in docs/v4-design.md §7 + the M-096 ledger notes in this diff). TWFE's unit auto-cluster stays; MPD's warn-and-analytical-fallback stays (n_bootstrap-independent).
  • Fixed: DiD/TWFE never cleared per-fit bootstrap state, so a wild fit followed by set_params(inference="analytical") + refit reported stale inference_method="wild_bootstrap" + n_bootstrap/n_clusters/p_val_type metadata. Both now reset _bootstrap_results at the top of fit().
  • Docs sweep: REGISTRY WCR fail-closed Note + MPD wild-availability corrections (SE-summary table + section bullet); troubleshooting.rst/choosing_estimator.rst examples gain the cluster= prerequisite (plus the adjacent weight_type=bootstrap_weights= kwarg fix); README/llms.txt/llms-practitioner.txt normative wild recommendations qualified DiD-vs-TWFE and the two CS-adjacent advice strings reworded; llms-full.txt DiD block annotated; the wild-bootstrap cluster-count guidance harmonized to the single 50-cluster convention (choosing_estimator.rst was the <30 outlier). A systematic repo-wide wild-mention disposition sweep was run: normative recommendations qualified, capability/method prose untouched.
  • Bookkeeping: docs/doc-deps.yaml gains troubleshooting.rst + llms-practitioner.txt under estimators.py (dependencies this diff proves); TODO row for the type-blind n_bootstrap holes in the already-validated estimators (HAD bool, dCDH bool+float, TROP float, SyntheticDiD float/jackknife); DEFERRED MPD anchor corrected; ContinuousDiD enrolled in the BAD_VALUES rollback lane (pre-existing catalog gap).

Methodology references (required if estimator / math changes)

  • Method name(s): Wild Cluster Restricted (WCR) bootstrap - inference selection policy only; the WCR kernel, weighting, CR1 SEs, p-values, and test-inverted CIs are unchanged. Bit-identical results for every previously-legal, non-degenerate configuration.
  • Paper / source link(s): Cameron, Gelbach & Miller (2008); Roodman, MacKinnon, Nielsen & Webb (2019) fwildclusterboot::boottest - see docs/methodology/REGISTRY.md "Wild cluster bootstrap (WCR)".
  • Any intentional deviations from the source (and why): None to the procedure. The fail-closed selector contract (cluster required; n_bootstrap >= 2 under wild) is documented as a labeled Note in the REGISTRY WCR section; the < 2 floor amendment to the locked v4-design decision is recorded in docs/v4-design.md §7 and the M-096 ledger notes in this same diff.

Validation

  • Tests added/updated: tests/test_v4_inference_policy.py (new, 126 tests - both rows' test_ref: validation sweep with set_params rollback atomicity, selector value-set incl. a non-string probe, cluster/floor coherence with full-message pins, boundary n=2 acceptance with a finite inference quintet, survey/Conley precedence at sub-floor counts on both estimators, MPD carve-out at n=0/1/999, refit metadata transitions, dynamic roster guard pinning inference exposure to exactly {DiD, MPD, TWFE}); tests/test_wild_bootstrap.py (the by-design flip); tests/test_estimators_vcov_type.py (cluster added to the wild/vcov coexistence pin); tests/test_base_estimator.py (ContinuousDiD BAD_VALUES enrollment).
  • Backtest / simulation / notebook evidence (if applicable): N/A - no numerical changes; both tutorial notebooks using wild bootstrap already pass cluster=. Sphinx -W build + the four post-build HTML guards pass; test_doc_snippets.py executes the corrected RST examples.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

…or (2(d) PR-B, M-081/M-096)

M-081: a shared utils.validate_n_bootstrap (promoted verbatim from
ChangesInChanges' local validator - non-negative int, numpy integers
accepted, bool/None/float/negative rejected) now runs at __init__ for
CallawaySantAnna, SunAbraham, EfficientDiD, ImputationDiD, TwoStageDiD,
WooldridgeDiD, ContinuousDiD, StaggeredTripleDifference and the DiD
family (DiD's __init__; MPD/TWFE inherit); CiC/QDiD re-point to the
shared helper with a byte-identical message. 0 stays legal and still
means bootstrap off on every > 0-gated analytical lane; no numeric
defaults change. Named exception (ledger + design doc): HAD keeps its
>= 1 floor - its n_bootstrap powers only the optional sup-t band whose
off-switch is fit(cband=False).

M-096: the inference selector fails closed. The accepted set is exactly
{"analytical", "wild_bootstrap"}, string-typed (an isinstance guard
rejects the one-element-ndarray hole in bare tuple membership), at
__init__ and transactional set_params. At fit - placed AFTER the survey
and Conley front doors so their NotImplementedError rejections keep
precedence - DiD with wild_bootstrap and no cluster= raises ValueError
(previously a SILENT analytical fallback; the pinned test
test_did_wild_bootstrap_requires_cluster flips BY DESIGN), and DiD/TWFE
with n_bootstrap < 2 raise (n_bootstrap in {0, 1} ran WCR with too few
draws and returned a wild-labeled all-NaN inference tuple; the < 2
floor amends the locked < 1, recorded in v4-design section 7 + the
M-096 notes in this diff). TWFE's unit auto-cluster stays; MPD's
warn-and-analytical-fallback stays (n_bootstrap-independent).

Fixed: DiD/TWFE never cleared per-fit bootstrap state, so a wild fit
followed by set_params(inference="analytical") + refit reported stale
inference_method="wild_bootstrap" + n_bootstrap/n_clusters/p_val_type
metadata. Both now reset _bootstrap_results at the top of fit().

Ledger: M-081 + M-096 planned -> done with test_ref
tests/test_v4_inference_policy.py (126 tests: the validation sweep with
rollback atomicity, the selector value-set incl. a non-string probe,
cluster/floor coherence with full-message pins, boundary n=2 acceptance
with a finite quintet, front-door precedence at sub-floor counts on
both estimators, the MPD carve-out at n=0/1/999, refit transitions, and
the dynamic roster guard pinning inference exposure to exactly
{DiD, MPD, TWFE}). ContinuousDiD enrolled in the BAD_VALUES rollback
lane (pre-existing catalog gap). Docs: REGISTRY WCR fail-closed Note +
MPD wild rows corrected (SE-summary table + section bullet);
troubleshooting/choosing_estimator examples gain the cluster=
prerequisite (plus the adjacent weight_type= -> bootstrap_weights=
kwarg fix); README/llms.txt/llms-practitioner normative wild
recommendations qualified DiD-vs-TWFE and the two CS-adjacent advice
strings reworded; llms-full DiD block annotated; a systematic
wild-mention disposition sweep recorded. The wild-bootstrap
cluster-count guidance is harmonized to the single 50-cluster
convention (choosing_estimator.rst was the <30 outlier). doc-deps gains
troubleshooting.rst + llms-practitioner.txt under estimators.py; TODO
row for the type-blind n_bootstrap holes in already-validated
estimators (HAD/dCDH/TROP/SyntheticDiD); DEFERRED MPD anchor corrected.
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Overall Assessment

Looks good — no unmitigated P0 or P1 findings.

Executive Summary

  • WCR numerical methodology, weighting, SEs, p-values, and CIs are unchanged.
  • The new fail-closed policy matches the labeled Registry note.
  • Validation, transactional set_params(), inference precedence, and refit-state clearing are consistently covered.
  • Deferred validation gaps are explicitly tracked.
  • Runtime tests were unavailable because dependencies are not installed; static parsing passed.

Methodology

  • P3 — Documented policy change (docs/methodology/REGISTRY.md:L106-L124)
    • Impact: None. DiD/TWFE WCR selection now requires coherent inputs; MPD’s analytical fallback remains documented. This does not alter the Cameron–Gelbach–Miller/Roodman et al. procedure.
    • Concrete fix: None required.

Code Quality

No findings. The shared validator and guards are consistently implemented in diff_diff/utils.py:L485-L501, diff_diff/estimators.py:L250-L257, and diff_diff/twfe.py:L280-L290.

Performance

No findings. Constructor validation and fit-time guards have negligible cost.

Maintainability

No findings. Centralizing n_bootstrap validation reduces duplicated contracts, and both affected fit implementations clear per-fit bootstrap state.

Tech Debt

  • P3 — Tracked exclusions (TODO.md:L68, DEFERRED.md:L107)
    • Impact: Type-blind validation in excluded estimators and MPD’s temporary fallback remain known limitations.
    • Concrete fix: Address through the existing TODO row and planned v4 MPD removal; no PR-blocking action required.

Security

No findings or apparent secrets/PII.

Documentation/Tests

No findings. Tests cover invalid types, rollback, selector values, DiD/TWFE prerequisites, survey/Conley precedence, MPD behavior, boundary counts, and refit transitions. Static AST parsing passed; runtime execution was unavailable due missing pytest and scientific Python dependencies.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant