feat(aggregation): EfficientDiD post-fit aggregate() replays the bootstrap on bootstrapped fits (M-023) - #783
Conversation
…strap on bootstrapped fits (M-023) Transplant the CS BootstrapReplaySpec mechanism (#780) onto EfficientDiD: post-fit aggregate('event_study'/'group') on a bootstrapped fit now REPLAYS the fit-time multiplier bootstrap from the kit-retained RNG state instead of raising NotImplementedError. Percentile se/CI/t match a fit-time aggregation to ~1 ULP (BLAS reassociation; p-values compared at 2/n_bootstrap); seed=None fits replay; pickles carry the state; post-fit set_params/mutation cannot alter a replay. The spec stamps the weight backend at capture (portable for stratified-survey / census-FPC / single-PSU degenerate branches); replays under a different Rust/NumPy backend and pre-replay legacy pickles fail closed. Replays re-emit the fit-time bootstrap warnings; the simple/total relays stay silent. The ES/group percentile-override appliers moved verbatim to diff_diff.bootstrap_utils (shared by the CS and EDiD fit paths and replays; oracle-verified bit-inert, plus a committed t == effect/se regression pin). Fixed: the EDiD bootstrap ES prep keyed horizons by raw t - g while the analytical aggregator buckets by int(t - g) — on fractional-period panels a strict sub-aggregate's percentile inference was attached to the pooled row (balance_e anchor filter included). All three keying sites now use the analytical expression (a no-op on integer panels); n_groups counts DISTINCT cohorts per bucket; fractional truncation-bucketing now emits a UserWarning, and a new REGISTRY truncation Note documents the full convention (double-width bucket 0, the PT-Post reference collision, cell-mass weighting). Docs/ledger: M-023/M-120/M-020 notes + code_refs, REGISTRY EDiD Note (b)/(c) + truncation Note + weight-backend/dCDH-M-026 trueups, migration-4.0, v4-design, all three llms guides, tutorial 15 prose, practitioner advice, CHANGELOG (Added + Fixed). TODO row retired; new df_survey-provenance cross-estimator row added. Tests: TestEfficientBootstrapReplay + Designs + TestEfficientFractionalPeriods (parity vs the native fit-time surface across plain/cluster/survey/stratified/ FPC/census-FPC/single-PSU/anticipation/alpha/mammen/covariate/PT-Post/ fractional designs; fail-closed gates; warning re-emission; retention with a spec-carrying kit), practitioner + t-recompute pins.
Overall assessment⛔ Blocker — one P0 inference bug. Executive summary
Methodology
Code QualityNo additional findings. Performance
MaintainabilityNo findings. Tech Debt
SecurityNo findings or apparent secrets. Documentation/Tests
Validation was limited to static inspection and AST parsing; the environment lacks pytest and NumPy. Path to Approval
|
… group CI replay parity CI review P0 (verified by execution): census-FPC zero-weight draws leave every multiplier-bootstrap replicate at the original effect, and np.std of a constant non-zero level can return a tiny positive value from mean-subtraction roundoff (~2.8e-17 at level 0.1), slipping past the se <= 0 guard and publishing an astronomically large finite t with a degenerate point CI. The shared percentile-statistic helpers (scalar and batch, consumed by every multiplier-bootstrap engine) now detect max(draws) == min(draws) and return the full NaN inference tuple with the existing zero-SE RuntimeWarning. Exact check, no tolerance: genuinely varying draws are unaffected. Tests: direct scalar/batch guard pins (constant-nonzero NaNs, near-constant unaffected, healthy batch column intact); the census-FPC replay arm now asserts full-NaN se/t/p/CI on BOTH the fit-time and replayed ES + group surfaces; group replay parity now also compares both CI bounds (CI review P2). REGISTRY Survey-Aware Bootstrap Note + CHANGELOG Fixed entry added.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0/P1 findings. Executive summary
Methodology
The replay is consistent with the bootstrap approach documented in Code QualityNo findings. The previous P0 is addressed in both helpers at Performance
MaintainabilityNo findings. Shared percentile-override helpers remove CS/EDiD implementation duplication. Tech Debt
SecurityNo findings or apparent secrets. Documentation/TestsNo findings. Group CI parity is covered at Focused tests could not run because pytest and NumPy are unavailable. AST parsing of the changed Python files passed. |
…ate rows (CI OpenBLAS failure) CI failed on ubuntu/windows/pure-python (macOS green) in the census-FPC replay arm: the zero-weight fused GEMM leaves every replicate row of the per-cell draw matrix IDENTICAL and exactly at the original effects, but the SECOND-STAGE re-aggregation matvecs (EDiD overall/ES/group; CS group) can reduce different row positions in different BLAS orders - identical rows in, rows differing by ~1 ULP out - producing a not-exactly-constant derived distribution whose ~1e-17 std slips past the zero/constant guards (observed se=1.37e-17 on ubuntu; Accelerate happens to reduce all rows identically, which is why macOS passed). Fix in both engines: when the replicate rows are identical (zero information), compute each second-stage reduction ONCE and broadcast, so the derived distributions are exactly constant on every platform and the existing zero/constant NaN guards fire deterministically. The fused-GEMM cell/overall/ES columns are exact (0*x sums are order-independent) and need no handling; CS's exposure was its group matvec (the same class, pre-existing since the census-FPC path shipped). No behavior change off the degenerate path: the broadcast branch requires bitwise-identical replicate rows, unreachable with any varying draw.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0/P1 findings. Executive summary
Methodology
Code QualityNo findings. The prior P0 is addressed by:
Performance
MaintainabilityNo findings. Moving both percentile-override paths into shared helpers removes CS/EDiD implementation duplication. Tech Debt
SecurityNo findings or apparent secrets. Replay retention uses factorized design arrays rather than raw unit labels. Documentation/TestsNo findings. Census-FPC full-NaN behavior is covered at Runtime tests could not be executed because NumPy, pandas, and pytest are unavailable. AST parsing of the 15 affected Python/test files passed. |
1 participant
Summary
aggregate('event_study')/aggregate('group')now work on bootstrapped EfficientDiD fits by REPLAYING the fit-time multiplier bootstrap from a kit-retainedBootstrapReplaySpec(the CS mechanism from feat(aggregation): CS post-fit aggregate() replays the multiplier bootstrap on bootstrapped fits (M-020) #780, transplanted): the RNG state is snapshotted at weight-stream construction and carried BY VALUE, so percentile se/CI/t match a fit-timefit(aggregate=...)aggregation to BLAS reassociation (~1 ULP,assert_allclose— never bit-identity; percentile p-values are count statistics compared at2/n_bootstrap).seed=Nonefits replay, pickles carry the state, and post-fitset_params/attribute mutation cannot alter a replay."portable". Replays under a mismatched backend and pre-replay legacy pickles fail closed with explicit remedies. Replays re-emit the fit-time bootstrap warnings; the'simple'/'total'relays stay silent and unchanged.staggered_bootstrap.pytodiff_diff.bootstrap_utils(a structuralProtocolfor the container contract) and are now shared by the CS and EfficientDiD fit paths and replays — one implementation, no twin drift. The move was gated by a pre/post oracle on the unmodified tree (bit-identical) plus a committedt == effect/seregression pin.t - gwhile the analytical aggregator buckets byint(t - g), so on fractional-period panels a strict sub-aggregate's percentile inference was attached to the pooled analytical row (thebalance_eanchor filter had the same hole). All three keying sites now use the analytical expression — a no-op on integer-period panels. Companions:n_groupscounts DISTINCT cohorts per bucket (was a cell count that over-counted on fractional panels; this also moves the ESncolumn on analytical fractional fits), and every aggregation that truncation-buckets a fractional horizon (fit-time, post-fit, replay,hausman_pretest) now emits aUserWarningpointing at the new REGISTRY truncation Note.practitioner_next_stepsadvises the post-fit route on bootstrapped EDiD fits;to_dataframe/absent-surface hints trued up; DiagnosticReport needs no change (EDiD never routes through the derived-ES resolver — its PT check is the Hausman pretest).Methodology references (required if estimator / math changes)
docs/methodology/REGISTRY.mdEfficientDiD section; bootstrap pattern follows Callaway & Sant'Anna (2021) / the Rdidpackage as already documented.int(t - g)truncation-bucketing convention (pre-existing on the analytical path; the bootstrap prep is now aligned to it) is recorded as a new REGISTRY- **Note:**covering the double-width bucket 0, the PT-Post reference collision, and the cell-mass within-bucket weighting, with a runtimeUserWarning. The M-023/M-120/M-020 ledger notes were amended in the same diff; the cross-estimatorsurvey_metadata.df_surveyprovenance residual is filed as a new TODO row.Validation
tests/test_aggregate_contract.py(newTestEfficientBootstrapReplay,TestEfficientBootstrapReplayDesigns,TestEfficientFractionalPeriods— parity vs the NATIVE fit-time surface across plain/cluster/weights-only-survey/stratified/FPC/census-FPC/single-PSU/anticipation/alpha/mammen/covariate/PT-Post/balance_e/fractional designs, fail-closed backend + legacy gates, warning re-emission, seedless idempotence, pickle round-trip, mutation immunity, spec-carrying retention),tests/test_efficient_did.py(committedt == effect/serecompute pin; comment trueups),tests/test_practitioner.py(post-fit advice pin).Security / privacy