Skip to content

feat(v4): TWFE event-study mode absorbs MultiPeriodDiD - M-010/M-060/M-082 (phase 3(a)) - #755

Merged
igerber merged 1 commit into
mainfrom
feat/v4-3a-twfe-event-study
Aug 8, 2026
Merged

feat(v4): TWFE event-study mode absorbs MultiPeriodDiD - M-010/M-060/M-082 (phase 3(a))#755
igerber merged 1 commit into
mainfrom
feat/v4-3a-twfe-event-study

Conversation

@igerber

@igerber igerber commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • TWFE event-study mode [M-010]: TwoWayFixedEffects.fit(..., event_study=True, time=<calendar>, spec="within"|"pooled", reference_period=None, post_periods=[...]) returns the unified EventStudyResults surface natively. spec="pooled" is the MultiPeriodDiD design verbatim via the shared estimation core (bit-exact under matched cluster settings; unconditional in the unit-less repeated-cross-sections form); spec="within" (default) absorbs the unit FE and omits the spanned treatment main effect.
  • Core extraction: DifferenceInDifferences._fit_event_study_core is the relocated MultiPeriodDiD.fit body, parameterized (include_treatment_main, warn_legacy_reference_default, cluster_override, estimator_name, _frame_offset). MPD numerics, messages, and warning attribution are bit-identical pre/post extraction (attribution pins in the new suite).
  • Inference stack from day one: unit auto-cluster with the static carve-outs mirrored lane-for-lane (dropped on Conley - only explicit cluster= combines; never injected as a survey PSU; dropped for explicit one-way analytical families); inference="wild_bootstrap" raises (the WCR implementation covers the static ATT only - MPD's silent fallback is not carried over); no legacy reference-period warning.
  • Explicit calendar partition: post_periods= is REQUIRED (non-empty, duplicate-free) in event-study mode - the treatment boundary is not observable from the documented time-invariant ever-treated indicator, so the MPD midpoint default would silently guess the reference and partition. MPD itself keeps its documented default through 3.9. Event-study calls pass time=/unit= as keywords (signature slot 4 stays post= through the M-082 window).
  • Deprecations: MultiPeriodDiD (+ the EventStudy alias, which is the same class object [M-060]) warns on construction with the successor named; static TwoWayFixedEffects.fit(time=) renames to post= behind the established shim [M-082] - time= survives as the event-study calendar column (4.0 semantic enforcement is M-083). ~140 static time= keyword callers migrated repo-wide (receiver-resolved per site; legitimate time= params on CiC/QDiD/TripleDifference/MPD/placebo surfaces untouched).
  • Container provenance (M-092 amendment Add comprehensive code review for diff-diff library #5): EventStudyResults gains the authoritative post_periods calendar partition (content-validated; threaded by _from_mpd) and estimation_spec ({within, pooled}; TWFE producer only); both serialize through to_dict().
  • Consumer ports: HonestDiD and PreTrendsPower gain calendar container routes with native-route parity, including the reference-anchored Roth gamma-unit plumbing (string-label degradation reproduced, not bypassed). HonestDiD is geometry-scoped to chronological partitions per the Rambachan-Roth positional restriction system (the pre-existing native-route permissiveness is documented and deferred). Both calendar routes share the hardened container-vcov contract (duplicate/incomplete vcov_index fail loud; _validate_vcov_subblock, HonestDiD with allow_singular=False; warned diagonal fallback only when no matrix is stored), guard empty pre AND post retained sets, fail closed on invalid explicit pre_periods= selections in calendar chronology, and warn on string calendar labels (chronology unverifiable; sorted() order assumed, matching the fit's own rule). plot_event_study derives the pre/post split from the partition provenance with per-contiguous-run shading on both renderers. DiagnosticReport/BusinessReport explicitly reject the surface (previously a silent empty-applicability report / all-null headline); admission is a tracked backlog row.
  • Documentation: REGISTRY gains the TWFE "Event-study mode (3.9)" subsection, the staggered-adoption detection-limit and string-label-chronology Notes, the MPD deprecation Note, and a correction of the pre-existing singleton edge-case overclaim (singletons are RETAINED class-wide, matching fixest's default; reghdfe drops them - now a labeled Deviation-from-R Note, execution-verified on both paths). Ledger flips: M-010 shimmed/phase 5, M-082 shimmed, M-060 planned/phase 5, plus the M-092/M-093/M-011/M-080 cross-amendments; v4-design section 4.1 amended (keyword calls + required partition, dated). CHANGELOG, teaching surfaces (quickstart, choosing_estimator, troubleshooting, homepage table, executed api examples, practitioner tree, README line), and all four bundled guides migrated off MPD / static time=. Notebook refreshes deliberately deferred to the phase-4 migration-guide PR (user decision 2026-08-07).

Methodology references (required if estimator / math changes)

  • Method name(s): TWFE event-study mode (within + pooled specs); MultiPeriodDiD (deprecated); HonestDiD calendar admission; PreTrendsPower calendar admission
  • Paper / source link(s): Freyaldenhoven, Hansen, Perez & Shapiro (2021, NBER WP 29170); Wooldridge (2010) Ch. 10; Rambachan & Roth (2023, ReStud); Roth (2022, AER: Insights)
  • Any intentional deviations from the source (and why): documented in docs/methodology/REGISTRY.md with recognized labels - the day-one unit auto-cluster with its three static-mirror carve-outs; the event-study wild-bootstrap rejection; the required explicit post_periods= (no midpoint default); the staggered-adoption detection limit under time-invariant D_i; singleton retention (Deviation from R: reghdfe drops, fixest retains - diff-diff matches fixest); HonestDiD calendar geometry scoping vs the permissive native route (DEFERRED row); string-calendar-label chronology assumption (warned).

Validation

  • Tests added/updated: tests/test_v4_merge_mpd.py (new, 72 tests: the section 4.1 gate triple + within numerical gate, mode/rename validation, wild-raise precedence, auto-cluster carve-out behavioral pins, deprecation choreography, surface contract incl. a replicate-survey numerical lane, inference integrity incl. warning-attribution baselines and the singleton class-consistency pin, consumer parity/rejection/vcov-integrity lanes with mask-first NaN-safe assertions); tests/test_event_study_surface.py (partition/spec provenance fields); tests/test_base_estimator.py (deprecated-class carve-out); tests/test_naming_guard.py (forwarding-init-shim registry); tests/test_event_study_consumers.py, tests/test_estimators.py, plus the repo-wide time= -> post= migration across ~12 test files and 4 benchmark scripts
  • Backtest / simulation / notebook evidence (if applicable): within-vs-pooled divergence floors calibrated on seeded DGPs (unbalanced 0.2092, covariate 0.0214); singleton SE-shift measured on both static and ES paths (0.41019 -> 0.40962); docs build -W green with all four post-build HTML guards; test_doc_snippets executes the six migrated pages end-to-end

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

…M-082 (phase 3(a))

TwoWayFixedEffects.fit(..., event_study=True, time=<calendar>,
spec="within"|"pooled", reference_period=None, post_periods=[...])
estimates per-period treatment effects and returns the unified
EventStudyResults surface natively. spec="pooled" is the MultiPeriodDiD
design verbatim (shared estimation core - bit-exact under matched
cluster settings, unconditional in the unit-less repeated-cross-sections
form); spec="within" (default) absorbs the unit FE and omits the
spanned treatment main effect. MultiPeriodDiD (+ the EventStudy alias)
is deprecated with a construction FutureWarning naming the successor;
static fit(time=) is renamed to post= behind the M-030-style shim
(time= survives as the ES calendar column; 4.0 enforcement is M-083).

Core mechanics:
- DifferenceInDifferences._fit_event_study_core is the relocated
  MPD.fit body, parameterized (include_treatment_main,
  warn_legacy_reference_default, cluster_override, estimator_name,
  _frame_offset) - MPD numerics, messages, and warning attribution are
  bit-identical pre/post extraction (attribution pins included).
- The mode carries TWFE's inference stack from day one: unit
  auto-cluster with the static carve-outs mirrored lane-for-lane
  (dropped on Conley - no implicit spatial x unit product kernel; never
  injected as a survey PSU; dropped for explicit one-way analytical
  families), a day-one wild-bootstrap ValueError, and no legacy
  reference-period warning.
- post_periods= is REQUIRED (non-empty, duplicate-free) in ES mode: the
  treatment boundary is not observable from the documented
  time-invariant ever-treated indicator, so the MPD midpoint default
  would silently guess the reference/partition; MPD itself keeps its
  documented default through 3.9. ES calls pass time=/unit= as keywords
  (slot 4 stays post= through the M-082 window).
- EventStudyResults gains two provenance fields (M-092 amendment #5):
  the authoritative post_periods calendar partition (content-validated,
  threaded by _from_mpd) and estimation_spec ({within,pooled}, TWFE
  producer only); both serialize through to_dict().

Consumer ports:
- HonestDiD and PreTrendsPower gain calendar container routes with
  native-route parity: the partition provenance reconstructs exactly
  the inputs the native MPD branches read, including the
  reference-anchored Roth gamma-unit plumbing (string-label degradation
  reproduced, not bypassed). HonestDiD is geometry-scoped to
  chronological partitions (suffix post set, last-pre reference) per
  the Rambachan-Roth positional restriction system; the pre-existing
  native-route permissiveness is documented (REGISTRY Note) and
  deferred (DEFERRED.md row). Both calendar routes share the hardened
  container-vcov contract (duplicate/incomplete vcov_index fail loud,
  _validate_vcov_subblock, HonestDiD with allow_singular=False, warned
  diagonal fallback only when no matrix is stored), require finite
  effects alongside finite positive SEs where their native conventions
  do, guard empty pre AND post retained sets (the post guard added to
  the native HonestDiD sibling too), fail closed on invalid explicit
  pre_periods= selections in calendar chronology, and warn on string
  calendar labels (chronology unverifiable; sorted() order assumed,
  matching the fit's own rule).
- plot_event_study derives the pre/post split from the partition
  provenance with per-contiguous-run pre-shading on both renderers.
- DiagnosticReport and BusinessReport explicitly reject
  EventStudyResults (previously a silent empty-applicability report /
  all-null headline); admission is a tracked backlog row.

Sweeps and hygiene:
- ~140 static TWFE/DiD time= keyword callers migrated to post= across
  tests, docs, guides, and benchmarks (receiver-resolved per site;
  legitimate time= params on CiC/QDiD/TripleDiff/MPD/placebo surfaces
  untouched; deliberate shim-test usage retained).
- MPD deprecation shim: forwarding __init__ + import-time __signature__
  mirror keeps BaseEstimator introspection working; pyproject
  filterwarnings ignore for the suite-noise window; naming-guard
  _FORWARDING_INIT_SHIMS registry keeps MPD in DiD's init-sharing
  group.
- ES within + hc2/hc2_bm gains the static path's dense full-dummy
  memory preflight (column-presence guarded).
- REGISTRY: TWFE "Event-study mode (3.9)" subsection (designs,
  estimate-shift, auto-cluster + carve-outs, wild raise, explicit
  partition, staggered-adoption detection limit, string-label
  chronology); the pre-existing singleton edge-case overclaim corrected
  to the RETAINED behavior with a Deviation-from-R Note (reghdfe drops,
  fixest retains; execution-verified on both paths); MPD deprecation
  Note; HonestDiD positional-geometry Notes.
- v4-deprecations ledger: M-010 shimmed/phase 5, M-082 shimmed, M-060
  planned/phase 5 (warning rides the parent), M-092 amendment #5, M-093
  admission amendment, M-011/M-080 cross-notes; v4-design section 4.1
  keyword + required-partition amendments (dated, same-diff).
- tests/test_v4_merge_mpd.py (72 tests): the section 4.1 gate triple +
  the within numerical gate, mode/rename validation, wild-raise
  precedence, auto-cluster carve-out behavioral pins, deprecation
  choreography, surface contract (incl. replicate-survey numerical
  lane), inference integrity (rank-deficiency NaN tuple, attribution
  baselines, singleton class-consistency), and consumer lanes
  (parity, geometry/provenance/vcov-integrity rejections, string-label
  warning, explicit-selection validation, plot geometry on both
  renderers). Parity assertions are mask-first (_eq_with_nans /
  _close_with_nans) so NaN-vs-zero regressions cannot be equated.
- Teaching surfaces migrated off MultiPeriodDiD / static time=:
  quickstart, choosing_estimator, troubleshooting, homepage estimator
  table, api pages (executed examples), practitioner decision tree,
  README catalog line, and all four bundled guides; docs build -W green
  with all post-build HTML guards.
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Overall assessment: ✅ Looks good

Executive summary

  • No unmitigated P0 or P1 findings.
  • TWFE within/pooled designs match the Registry equations and cited methodology.
  • Inference consistently uses safe_inference; empty and invalid covariance paths fail closed.
  • Intentional deviations are explicitly documented in the Methodology Registry.
  • Remaining limitations are tracked in TODO.md or DEFERRED.md.

Methodology

  • Severity: P3 — informational
    Impact: Unit auto-clustering, required post_periods, wild-bootstrap rejection, singleton retention, and staggered-adoption detection limits are documented deviations, not defects.
    Concrete fix: None required.
    Locations: docs/methodology/REGISTRY.md:L397-L458, docs/methodology/REGISTRY.md:L613-L626

The TWFE specifications align with Freyaldenhoven et al. and Wooldridge. HonestDiD and PreTrendsPower preserve the Rambachan–Roth and Roth covariance/reference-period contracts.

Code Quality

No findings.

Performance

No findings. The dense HC2/HC2-BM path includes an appropriate memory warning at diff_diff/twfe.py:L1136-L1169.

Maintainability

  • Severity: P3 — informational
    Impact: The deprecated MultiPeriodDiD forwarding constructor loses static IDE/type-checker argument validation during the 3.9 window. Runtime validation remains intact.
    Concrete fix: None required before its planned 4.0 removal; tracked in DEFERRED.md.
    Location: DEFERRED.md:L130

Tech Debt

  • Severity: P3 — informational
    Impact: Serialized EventStudyResults lacks variance-family and clustering provenance. Statistical values remain correct, but detached results cannot identify their inference configuration.
    Concrete fix: Add the cross-producer provenance fields described in the tracked backlog entry.
    Location: TODO.md:L26

Security

No findings or apparent secrets/PII.

Documentation/Tests

  • Severity: P3 — informational
    Impact: Internal shared-core parity tests would not detect a defect common to both implementations.
    Concrete fix: Add the tracked committed fixest::feols event-study golden.
    Location: TODO.md:L73

Test execution was unavailable because the review environment lacks pytest and runtime dependencies.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Aug 8, 2026
@igerber
igerber merged commit be694f4 into main Aug 8, 2026
45 of 46 checks passed
@igerber
igerber deleted the feat/v4-3a-twfe-event-study branch August 8, 2026 12:09
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