Skip to content

feat(v4): ImputationDiD + TwoStageDiD post-fit aggregate() via panel-backed kits (2(b) PR-3b, M-021/M-022/M-118/M-119) - #749

Merged
igerber merged 2 commits into
mainfrom
feat/v4-imputation-twostage-aggregate-kit
Aug 5, 2026
Merged

feat(v4): ImputationDiD + TwoStageDiD post-fit aggregate() via panel-backed kits (2(b) PR-3b, M-021/M-022/M-118/M-119)#749
igerber merged 2 commits into
mainfrom
feat/v4-imputation-twostage-aggregate-kit

Conversation

@igerber

@igerber igerber commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Deprecates fit(aggregate=, balance_e=) on ImputationDiD and TwoStageDiD (3.9, removed 4.0; ledger rows M-021/M-022 flip to shimmed, new rows M-118/M-119 claim the reserved balance_e slots) in favor of post-fit results.aggregate('event_study'/'group'/'simple', balance_e=) on the shared mixin contract - the CS/EfficientDiD register: a plain fit() never warns; supplying ANY value (None included) warns once; the deprecated path returns exactly the numbers it always did; the imputation_did/two_stage_did wrappers forward the shared sentinel so plain wrapper calls never warn.
  • Both estimators are panel-backed recomputers: the aggregation + variance engines move verbatim into new import-leaf modules imputation_aggregation.py / two_stage_aggregation.py (imputation.py 2,961→1,491 lines; two_stage.py 3,551→2,430, exiting the "consider splitting" band), inherited by the estimators (fit-time byte-identical) and hosted post-fit by throwaway kit aggregators populated exclusively from kit snapshots (public-field mutation cannot reach recompute - pinned).
  • Memory contract: ImputationDiD's kit references the SAME per-fit objects _fit_data already retains for pretrend_test() - zero marginal memory, pickle +392 B via memoization. TwoStageDiD's kit is the FIRST panel retention on its results: a deduplicated column-subset copy of the working frame (+170 KB plain / +434 KB replicate on the 3,000-row benchmark; store_kit opt-out tracked in DEFERRED.md). Fit timing unchanged within noise (matched pre/post bands overlap).
  • Inertness gate: post-fit output equals fit-time surfaces at 1e-14 across plain/covariate/cluster (incl. cluster= naming the unit column)/anticipation/horizon_max/pretrends/Prop-5/survey-TSL/always-treated-pad/replicate (healthy, dropped, undefined-df) fits x balance_e {0,1,2}. Replicate arms are LEVEL-MATCHED (aggregate(L)fit(aggregate=L); the deprecated joint-stack coupling of the OVERALL row is a documented migration delta on degenerate designs, pinned). TwoStage post-fit containers reproduce the M-092 vcov/index/df contract exactly.
  • Bootstrapped fits fail closed at every level including 'simple' (uniform CS/EDiD parity; replay tracked in TODO.md); a fit whose bootstrap FAILED aggregates normally. Container admission NOT widened: ImputationDiD rejected BY DESIGN (no joint ES covariance); TwoStageDiD DEFERRED pending a normalization derivation (DEFERRED.md paper-gated row) - both consumer TypeErrors state the grounds.
  • Behavior deltas beyond the shim: (1) ImputationDiD's M-127 df_convention inert-config warning predicate is now REACHABILITY-based (post-fit aggregate('event_study') reaches the pretrends lead inference on analytical fits); (2) fixed a pre-existing TwoStageDiD corner where an event study with EVERY non-reference horizon Proposition-5-unidentified returned a reference-only surface, dropping the all-NaN Prop-5 rows and their warning.
  • Ledger amendments M-092 (qualifier extends to ImputationDiD) / M-093 (fourth amendment) / M-127; matrix floor 118→120; ~70 new contract tests incl. wrapper balance_e pins, the all-Prop-5 regression, covariate arms, and consumer rejection pins on REAL containers; 199-site deprecated-call-site sweep via message-scoped filterwarnings marks; full docs sweep (llms x3, README, practitioner handlers, api rst + stubs, REGISTRY notes (a)-(e) for both + M-127 amendment, variance-conventions, CHANGELOG, dev-status, doc-deps, TODO/DEFERRED rows, citation retargets); tutorials 11/12/15 migrated to post-fit and re-executed.

Methodology references (required if estimator / math changes)

  • Method name(s): Borusyak-Jaravel-Spiess (2024) imputation DiD (Theorem 3 conservative variance, Proposition 5); Gardner (2022) two-stage DiD (joint GMM sandwich); Rambachan-Roth HonestDiD admission analysis (deferred).
  • Paper / source link(s): Borusyak, Jaravel & Spiess (2024), Review of Economic Studies 91(6) (doi:10.1093/restud/rdae007); Gardner (2022), arXiv:2207.05943; see docs/methodology/REGISTRY.md §ImputationDiD / §TwoStageDiD.
  • Any intentional deviations from the source (and why): No numerical changes to the estimators - the moves are verbatim and post-fit recompute is pinned inert at 1e-14. Documented decisions in the new REGISTRY Notes (rows M-021/M-118 and M-022/M-119): the balanced-window balance_e rule (each estimator's own pre-existing rule, divergent from CS/EfficientDiD's anchor-horizon rule), panel-backed kit retention (memory contract), level-matched replicate replay semantics + the overall-row migration delta, uniform bootstrap fail-close, per-row df_used (Imputation) vs scalar df relay (TwoStage), the M-127 reachability predicate, and the admission decisions (Imputation by-design; TwoStage deferred).

Validation

  • Tests added/updated: tests/test_aggregate_contract.py (two new estimator blocks, ~70 tests), tests/test_event_study_consumers.py (rejection classes on real containers), tests/test_practitioner.py, tests/test_guides.py, tests/test_v4_matrix.py, tests/test_naming_guard.py, plus the 199-site sweep and monkeypatch-target repoints across 16 existing test files (test_imputation.py, test_two_stage.py, methodology/survey/pretrends/replicate suites; spillover sites deliberately untouched and verified).
  • Backtest / simulation / notebook evidence (if applicable): inertness matrix at assert_allclose 1e-14 (documented in the REGISTRY notes and pinned in the contract tests); R-parity and Stata-golden suites unchanged; tutorials 11_imputation_did.ipynb, 12_two_stage_did.ipynb, 15_efficient_did.ipynb migrated and re-executed clean via nbmake.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

…backed kits (2(b) PR-3b, M-021/M-022/M-118/M-119)

Deprecates fit(aggregate=, balance_e=) on both estimators (3.9, removed
4.0) in favor of post-fit results.aggregate('event_study'/'group'/
'simple', balance_e=) on the shared M-122 mixin contract - the
CS/EfficientDiD register: a plain fit() never warns; supplying ANY
value (None included) warns once via the shared NOT_SUPPLIED sentinel;
the deprecated path returns exactly the numbers it always did; the
imputation_did/two_stage_did wrappers forward the sentinel so plain
wrapper calls never warn.

Both estimators are PANEL-BACKED recomputers (no compact influence
payload can honor a different balance_e): the aggregation + variance
engines move verbatim into new import-leaf modules
imputation_aggregation.py / two_stage_aggregation.py (imputation.py
2961 -> 1491 lines; two_stage.py 3551 -> 2430, exiting the splitting
band), inherited by the estimators (fit-time byte-identical) and hosted
post-fit by throwaway kit aggregators populated exclusively from kit
snapshots. ImputationDiD's kit references the SAME per-fit objects
_fit_data already retains for pretrend_test() - zero marginal memory,
pickles +392 B via memoization. TwoStageDiD's kit is the first panel
retention on its results: a deduplicated column-subset copy of the
working frame (documented memory-contract change; +170 KB plain /
+434 KB replicate on the 3,000-row benchmark; store_kit opt-out
tracked in DEFERRED.md).

Inertness gate: post-fit output equals the fit-time surfaces at 1e-14
across plain/covariate/cluster (incl. cluster=unit-column)/
anticipation/horizon_max/pretrends/Prop-5/survey-TSL/always-treated-
pad/replicate (healthy, dropped, undefined-df) fits and balance_e
{0,1,2}, with replicate arms LEVEL-MATCHED: compute_replicate_refit_
variance validates replicates jointly, so aggregate(L) reproduces
fit(aggregate=L) exactly and the deprecated fit(aggregate=)'s coupling
of the OVERALL row to the joint stack is a documented migration delta
on degenerate designs (pinned by contract tests). TwoStage post-fit
event-study containers reproduce the M-092 contract exactly (joint
GMM vcov + index + df analytical; vcov=None + replayed df replicate).
Bootstrapped fits fail closed at every level including 'simple'
(uniform CS/EDiD parity; replay tracked in TODO.md); a fit whose
bootstrap FAILED aggregates normally. Imputation pretrends+replicate
post-fit ES raises the same NotImplementedError the fit-time gate
raises.

Behavior deltas beyond the shim: (1) ImputationDiD's M-127
df_convention inert-config warning predicate is now REACHABILITY-based
(post-fit aggregate('event_study') reaches the pretrends lead
inference on analytical fits); (2) fixed a pre-existing TwoStageDiD
corner where an event study with EVERY non-reference horizon
Proposition-5-unidentified returned a reference-only surface, dropping
the all-NaN Prop-5 rows and their warning. Container admission into
honest/pretrends is NOT widened: ImputationDiD rejected BY DESIGN (no
joint ES covariance); TwoStageDiD DEFERRED pending a normalization
derivation (pre-period coefficients are stage-1 residual means while
HonestDiD hard-codes delta_0=0) - both TypeErrors state the grounds;
DEFERRED.md carries the paper-gated row.

Ledger: M-021/M-022 flip to shimmed; new rows M-118/M-119 claim the
reserved balance_e slots (balanced-window rule documented, divergent
from CS/EDiD's anchor-horizon rule); M-092 qualifier extends to
ImputationDiD; M-093 fourth amendment; M-127 amendment; floor 118 ->
120. Tests: two new contract-test blocks (~70 tests incl. wrapper
balance_e pins, the all-Prop-5 regression, covariate arms), consumer
rejection classes on REAL containers, practitioner/guides pins,
monkeypatch-target repoints for the moved solver globals (spillover
sites deliberately untouched), and a 199-site deprecated-call-site
sweep via message-scoped filterwarnings marks. Docs: llms x3, README
step-7 line, practitioner handlers, api rst + stubs, REGISTRY notes
(a)-(e) for both estimators + M-127 amendment, variance-conventions,
CHANGELOG, dev-status, doc-deps, TODO/DEFERRED rows, citation
retargets (re-derived, CHANGELOG history exempt); tutorials 11/12/15
migrated to post-fit and re-executed.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Overall assessment

Looks good — no unmitigated P0 or P1 findings.

Executive summary

  • Affected methods: BJS ImputationDiD and Gardner TwoStageDiD.
  • Aggregation and variance engines remain mathematically unchanged apart from the documented TwoStage Proposition-5 output fix.
  • safe_inference() consistently protects t-statistics, p-values, and confidence intervals.
  • Empty windows, bootstrap limitations, replicate replay, and container-admission decisions are explicitly handled or documented.
  • No security or secret exposure found.

Methodology

  • Severity: P3 — informational
    Impact: Balanced-window semantics, level-matched replicate replay, bootstrap fail-close behavior, and HonestDiD admission decisions are intentional, documented deviations or limitations. They agree with the cited BJS and Gardner reviews. See docs/methodology/REGISTRY.md:L1870, docs/methodology/REGISTRY.md:L1959.
    Concrete fix: None required.

  • Severity: P3 — informational
    Impact: The TwoStage all-Proposition-5 case now correctly retains unidentified horizons with all-NaN inference and n_obs > 0, matching the BJS identification restriction. See diff_diff/two_stage_aggregation.py:L487-L546 and docs/methodology/REGISTRY.md:L1930.
    Concrete fix: None required.

Code Quality

  • Severity: None
    Impact: No new inline inference anti-patterns, partial NaN guards, or incomplete parameter propagation found.
    Concrete fix: None.

Performance

  • Severity: P3 — informational, tracked in DEFERRED.md
    Impact: TwoStageDiD now retains an O(n_obs) panel subset; replicate designs may additionally retain an O(n_obs × R) matrix.
    Concrete fix: None for this PR; the proposed store_kit=False opt-out is tracked at DEFERRED.md:L73.

Maintainability

  • Severity: None
    Impact: Aggregation engines were cleanly extracted into import-leaf modules while compatibility exports were preserved.
    Concrete fix: None.

Tech Debt

  • Severity: P3 — informational, tracked
    Impact: Post-fit bootstrap replay and DiagnosticReport consumption of post-fit containers remain deferred without silently changing estimates. See TODO.md:L25-L26.
    Concrete fix: None required for approval.

Security

  • Severity: None
    Impact: No secrets, private keys, or unsafe data-handling changes detected.
    Concrete fix: None.

Documentation/Tests

  • Severity: P3 — informational
    Impact: Coverage includes empty windows, Proposition-5 rows, bootstrap failure gates, survey/replicate df propagation, pickle round-trips, and public-field isolation. Static parsing and git diff --check passed. Runtime tests could not be executed because this review environment lacks pytest.
    Concrete fix: Ensure CI runs the changed aggregation and methodology suites with project dependencies installed.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Aug 4, 2026
…it-identity (CI matrix)

The healthy-design arm of test_replicate_overall_row_migration_delta
(both estimator twins) asserted BIT-identical overall_se between a
plain fit and a fit(aggregate='group') fit. The two replicate-variance
computations route their matmul through different BLAS kernel shapes
([overall] vs [overall, groups] stacked layouts), which reassociates at
~1 ULP - bit-identical on macOS/Accelerate, not on OpenBLAS-ARM
(ubuntu-24.04-arm py3.11/3.13/3.14) or Windows (py3.13/3.14). Compare
at rtol=1e-12: a REAL replicate-drop coupling delta is O(se) itself,
far above the band, so the pin's discriminating power is unchanged.
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 1c5a67bc6da67f38bfa31a74527e0c9234906d75


Overall assessment

Looks good — no unmitigated P0 or P1 findings.

Executive summary

  • Affected methods are BJS ImputationDiD and Gardner TwoStageDiD.
  • Post-fit aggregation preserves the documented weighting and variance methodologies.
  • Empty windows, unidentified horizons, invalid SEs, bootstrap paths, and parameter propagation are handled consistently.
  • Intentional replay and container-admission differences are documented in the Methodology Registry.
  • No security or secret exposure found.

Methodology

  • Severity: P3 — informational
    Impact: Balanced-window semantics, replicate replay, bootstrap fail-close, df propagation, and HonestDiD/pretrends admission decisions are intentional and documented. The aggregation engines remain consistent with BJS Theorem 3/Proposition 5 and Gardner’s joint-GMM sandwich. See docs/methodology/REGISTRY.md:L1865 and docs/methodology/REGISTRY.md:L1925-L1960.
    Concrete fix: None required.

  • Severity: P3 — informational
    Impact: TwoStageDiD now retains all-NaN Proposition-5-unidentified horizons rather than returning a misleading reference-only surface. See diff_diff/two_stage_aggregation.py:L480-L656.
    Concrete fix: None required.

Code Quality

  • Severity: None
    Impact: No new inline inference computations, partial NaN guards, incomplete parameter propagation, or semantic-contract violations found. See diff_diff/imputation_results.py:L233-L445 and diff_diff/two_stage_results.py:L234-L447.
    Concrete fix: None.

Performance

  • Severity: P3 — tracked in DEFERRED.md
    Impact: TwoStageDiD retains an O(n) panel subset and, for replicate designs, an O(n×R) replicate matrix.
    Concrete fix: None for this PR; the proposed kit-retention opt-out is tracked at DEFERRED.md:L70-L75.

Maintainability

  • Severity: None
    Impact: The extracted aggregation modules preserve estimator behavior while reducing the size of the primary estimator modules.
    Concrete fix: None.

Tech Debt

  • Severity: P3 — tracked
    Impact: Post-fit aggregation for bootstrapped fits and DiagnosticReport consumption of post-fit event-study containers remain deferred without silently changing estimates. See TODO.md:L23-L27.
    Concrete fix: None required for approval.

Security

  • Severity: None
    Impact: No credentials, secrets, unsafe deserialization, or new external-input execution paths found.
    Concrete fix: None.

Documentation/Tests

  • Severity: P3 — informational
    Impact: Contract coverage includes empty windows, unidentified horizons, bootstrap gates, replicate replay, survey df propagation, and mutation isolation. Runtime tests could not be executed because the review environment lacks required dependencies; static parsing of central changed files succeeded.
    Concrete fix: Ensure CI runs the changed aggregation, methodology, replicate, and consumer suites.

@igerber
igerber merged commit cf6bb75 into main Aug 5, 2026
31 checks passed
@igerber
igerber deleted the feat/v4-imputation-twostage-aggregate-kit branch August 5, 2026 09:51
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