fix(v4): ETWFE reference-period normalization (#724) + Stata jwdid/csdid parity - #729
Merged
Conversation
…did parity Fixes issue #724 and anchors both staggered estimators to their canonical Stata implementations. Eleven local review rounds turned up further identification defects in the same code, including a P0; those are fixed here too. ## The #724 fix With `control_group="never_treated"`, the saturated ETWFE design emitted every cohort x time indicator INCLUDING the reference period -- collinear with the absorbed unit fixed effects -- and left generic QR rank detection to remove whichever column it picked. On the real `mpdta` panel that silently removed ATT(2004, 2004) and ATT(2006, 2007), returning 5 of 7 post-treatment cells. The reference is not a free choice: W2025 Eq. 6.1 excludes `dg_i·f(g-1)_t` "so that s = g-1 is the reference period", and Stata `jwdid ... never` omits the same cell. The design now omits `ref(g)` explicitly -- per-cohort and weight-aware, since a panel-wide rule omits an all-zero column when a cohort is unobserved there and leaves the collinearity intact. The surviving pre-fix estimates were not merely incomplete, they were measured against the wrong baseline. ATT(2007,2007) by hand against references 2006/2005/2004/2003 gives -0.0261 / -0.0571 / -0.0599 / -0.0294; the library returned -0.0571, i.e. a correct 2x2 DiD against a silently chosen 2005. Not confined to `never_treated`: any cohort whose cell block spans its own cohort dummy hits it, which `anticipation > 0` makes reachable on the default `not_yet_treated` path. ## Stata parity arm `benchmarks/stata/generate_etwfe_cs_golden.do` anchors ETWFE and CallawaySantAnna against `jwdid` / `csdid` on the committed `mpdta` panel (pinned SHA-256, so CI needs neither Stata nor network). This REPLACES a test that validated nothing: `test_wooldridge.py` asserted ETWFE == CS within 5e-3, which is false on real data (0.0171 at (2007,2007)) and passed only because `load_mpdta()` was silently serving a synthetic effect-homogeneous DGP when its source 404'd (issue #722). Stata's own two implementations reproduce the same disagreement, establishing it as an estimator property rather than a bug. The arm also surfaced a real deviation: ETWFE `hc1` SEs are uniformly BELOW `jwdid`'s (anti-conservative), ~0.1% at G=500 and ~2.8% at G=20. The mechanism is NOT identified -- it tracks sqrt(G/(G-1)) but sits above it, and `solve_ols` already applies the full CR1 -- so it is recorded as measured, not diagnosed, and pinned rather than "corrected" with an unverified factor. The never-treated arm shows the SAME factor over a different cell count, which rules out explanations keyed to the comparison set. ## Identification defects found while reviewing the above - P0: partial rank loss returned MISLABELED contrasts as ATTs. When some treatment columns were dropped, the survivors were re-identified as whatever contrast the reduced design supports and kept their ATT(g,t) labels. Measured with true effects 1.0 and 3.0: ATT(3,5) came back as -1.9393 -- wrong sign -- inside overall_att = +0.7703 against a true ~1.8, with NO warning under `rank_deficient_action="silent"`. Pre-existing and worse at base. A fit now fails closed unless every requested cell survives. - Within-cohort support CONNECTIVITY: per-period support is necessary but not sufficient; identification runs through the unit FE. A cohort whose units split into disjoint observation windows had a genuine post cell dropped (`g4_t5`) with the overall ATT computed from the survivor. OLS-only -- the nonlinear paths use explicit dummies and are unaffected. - A fit no longer returns a NaN overall ATT as success (point estimate only; NaN SE stays legitimate where only the variance is unidentifiable). - Invalid survey weights (NaN / negative / Inf, fractional fweight) and invalid strata/PSU/FPC could hide in rows that cohort exclusion deleted, yielding a confident finite estimate: 0.021416714008590323, with a warning blaming a missing pre-treatment period. The FULL design is now validated pre-exclusion. - `survey_design=` + unidentified-cohort exclusion is REFUSED: deleting rows drops their PSUs/strata from the TSL variance and from `df_survey = n_PSU - n_strata` (measured 22 -> 14, finite SE, no diagnostic). Domain estimation via `SurveyDesign.subpopulation()` is the correct answer and is tracked. - `rank_deficient_action="error"` was silently downgraded to a warning for skipped cells; all three modes now dispatch. ## Deliberate capability regression All-eventually-treated panels (no never-treated group) now RAISE. They previously returned finite, silently wrong ATTs -- on a true-1.5 DGP: ATT(3,8) = -0.0096, ATT(5,9) = -0.2393, overall_att = 1.0023. W2025 Section 5.4 defines the fix (last cohort as control, its columns dropped deliberately rather than by QR); the library already does this for the cohort-TREND column but not for the cells. Tracked at High priority; landing it restores these fits. Four existing tests asserted only that such values were FINITE, which is what let a P0 live in a tested path; they now assert the refusal with the measured numbers in their docstrings. ## Tracking Every refusal added here is classified terminal vs stopgap in TODO.md, with the stopgaps carrying what "done" means: survey domain estimation, anticipation-only estimand semantics, same-period comparison support, component-aware estimation, W2025 5.4 cell normalization, the SE-derivation subsample ladder, and the duplicate survey resolution. M-123/M-124 and the CHANGELOG record the user-visible behavior changes. Docs: REGISTRY normalization + connectivity + invariant notes, tutorial 16 (lead-and-lag vs lag-only, demonstrated rather than asserted), API page, constructor docstring, and the local paper review all corrected to match runtime behavior. Verification: 6992 passed across every WooldridgeDiD/SurveyDesign-touching suite; notebook re-validated with --nbmake; mypy 2.3.0 clean; black/ruff clean. Stata golden regenerated with local Stata (StataSE 19), verified zero numeric or key-set change -- only `meta.ssc_versions` moved.
Overall assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive summary
MethodologyDocumented Stata HC1 SE deviation
Documented all-eventually-treated limitation
The primary normalization itself is methodologically sound: Code QualityNo findings. All changed inference construction uses PerformanceDuplicate survey resolution
MaintainabilityNo findings. The changed interaction-matrix return contract is propagated to its callers, and diagnostics are centralized. Tech DebtDeferred estimator capabilities are explicitly tracked
SecurityNo findings. No credential, private-key, or secret patterns were detected in the changed sources or fixtures. Documentation/TestsFew-cluster Stata parity is not regression-pinned
The committed |
igerber
added a commit
that referenced
this pull request
Jul 27, 2026
…y-treated fits (W2025 Sec 5.4) (#730) * feat(v4): ETWFE per-period comparison support - restore all-eventually-treated fits Implements W2025 Section 5.4 for the cell and cohort-trend families: with no never-treated group the last cohort serves as the reference, so periods where no unit is untreated carry no identified ATT(g, t) and are removed from the estimation sample before the solve. Before #729 these panels returned finite and WRONG numbers (ATT(3,5) = -1.9393 against a true +1.0, inside overall_att = +0.7703 against ~1.8); #729 made them fail closed. They now fit, returning the absolute tau_gt on the retained range (measured: cells (3,3..7) and (5,5..7) on a {3,5,8} panel, overall_att = 1.5502 against a true 1.5, cohort 8 correctly receiving no cells). Support predicate, keyed on the regression baseline rather than the method: include_pre -> cohort == 0 only otherwise -> cohort == 0 OR cohort - anticipation > t The include_pre branch counts ONLY never-treated rows. Omitted reference cells sit in the baseline but do not identify the period: cohort h's indicator is absorbed by the unit FE, so the period dummy stays reproducible from the emitted cells and the design remains collinear. References are a one-shot fixed point on the not-yet-treated branch (reference eligibility IS the predicate's second disjunct), asserted at runtime. On the never-treated branch they can legitimately move, which renormalizes that cohort's ATTs, so that case warns by name. Nothing is dropped or renormalized silently: - warning (a) at filter time names the periods, the row count, and the branch-correct cause; it fires before anything downstream can raise - warning (b) after the final build names zero-cell and fully-dropped cohorts - neither is gated on rank_deficient_action Under survey_design= the filter refuses instead of subsetting: deleting rows removes their PSUs and strata from the TSL meat and from df_survey = n_PSU - n_strata. The refusal is conditional on rows actually dropping (1 of 59 survey fit calls in the suite) and points at explicit frame restriction, not SurveyDesign.subpopulation(), whose zero-weight padding _reject_zero_weight_groups refuses on the OLS path. results.groups and _n_g_per_cohort become cells-derived so a cohort retained only as a control is not advertised as estimated. The design-side readers of `groups` deliberately keep the present-cohort list: a cells-derived list there drops D_{G_max} x X, silently applying a covariate normalization this change defers (measured: 8/8 ATTs move, max 0.0648). The cohort_trends pre-period guard moves after filtering and exclusion, and iterates trend_groups rather than every present cohort - a zero-cell G_max receives no trend column, so demanding two pre-periods of it was a spurious refusal. Test audit re-pinned to measured behavior at eight sites; two unaffected sites (the cohort-count proxy pin, and the anticipation pseudo-control guard) verified unchanged. * test(v4): comparison-support coverage - Eq. 5.15 cell set, reference moves, groups-leak guard Adds TestComparisonSupportFiltering: Eq. 5.15 correspondence parameterized over anticipation 0/1/2 (supported set 7 -> 6 -> 5, with the references-is-None exclusion that removes cohort 3 at anticipation=2), the branch-1 reference-move warning and its no-move companion on a truncated frame, branch-2 reference invariance, warning (a) surviving a downstream raise, single-reporting of a zero-cell cohort that exclusion also removes, bootstrap on a filtered fit, and the regression guard that a cells-derived groups list has not leaked into the covariate design. * test(v4): Stata jwdid_alltreated parity arm - external W2025 Sec 5.4 anchor Adds a fourth golden arm: the pinned mpdta panel with never-treated counties dropped (191 units, 955 rows). Stata jwdid succeeds on it and estimates on 764 rows, because with no never-treated group the last cohort (2007) becomes the reference and the fully-treated periods carry no identified ATT. jwdid reports only the smaller N; the library computes the identical cell set on the identical rows and warns. Measured: cells (2004,2004), (2004,2005), (2004,2006), (2006,2006) in both, ATTs agreeing to ~1e-15, N = 764 in both. The arm runs plain jwdid, not 'jwdid ... never' -- there is no never-treated group left for the latter. It is wrapped in preserve/restore and placed before the emit block so the subset cannot reach the schema asserts or the three arms that share the in-memory dataset; verified by diffing the regenerated golden, which changes only by the new block plus its meta command key (csdid 24 cells, jwdid 8, jwdid_never 13 all bit-identical). The SE ratio is measured at THIS cluster count rather than inherited: G=191 gives 1.00264201, not the 1.001006 the sibling arms pin at G=500. Copying the sibling constant would have failed on first run. The observed sequence (1.0280@G=20, 1.0132@G=40, 1.00264@G=191, 1.0010@G=500) is monotone in G. * docs(v4): two-axis sweep for ETWFE comparison support - ledger M-125, REGISTRY, guides Falsification axis (claims this change makes false): - wooldridge.py: five stale in-file claims -- the 'tracked in TODO.md' and 'does not yet compute' diagnostic notes, both completeness-gate cause attributions (which named a condition the filter now removes automatically, and would have sent users to restrict the panel when the real cause is a covariate), the cohort_trends docstring, and the trend-block comment. - REGISTRY: the Section 5.4 deviation note is NARROWED rather than deleted -- the rule governs four regressor families and this change implements three, so deleting it would replace a too-broad note with a false one. Adds Notes for per-period support, the always-on reporting, reference movement, zero-cell cohorts, the second survey boundary, and the never_treated control-pool asymmetry (pre-existing, measured: on the nonlinear paths treated units' pre-treatment rows ARE the comparison, so ':2028' was wrong). - Ledger: M-125 added; M-124's capability-regression paragraph and M-123's exclusion-warning claim amended. (125, 125) is APPENDED to _INITIAL_ID_RANGES rather than widening (122, 124), per that file's own 'extend with a NEW tuple' rule -- both yield the same id set, so no test would have caught the violation. Stale 99/101 count prose refreshed in test_v4_matrix.py and v4-design.md; no phase-table edit (M-123/M-124 shipped with none, and test_spec_tokens_resolve enforces design->ledger only). - CHANGELOG, TODO (close one row, narrow two, widen one, add four), wooldridge_etwfe.rst, wooldridge_results.py, the paper review, survey-roadmap (two limitation rows -- the table listed no WooldridgeDiD entry at all). Additive axis (what a restored capability must document, which a falsification grep cannot reach): tutorial 16 gains a worked all-eventually-treated section, llms-full.txt gains cohort_trends, survey_design and the all-treated contract, llms-autonomous.txt gains the qualifications under 4.4. Opportunistic, pre-existing (falsified by #729, not by this change): METHODOLOGY_REVIEW.md and DEFERRED.md both claimed Stata parity was deferred pending a Stata install. The QMLE-weight bullet is deliberately LEFT OPEN -- every golden arm is linear jwdid, so no QMLE cluster-SE reference exists. Deliberately NOT changed: survey-roadmap.md's '12 of 16 estimators' line, which is scoped to replicate-weight variance -- WooldridgeDiD genuinely rejects those, so 'fixing' it would advertise a capability the library refuses. * fix(v4): harden ETWFE comparison support - cohort-share boundary, accurate cause reporting Correctness - `aggregate(weights="cohort_share")` now fails closed when comparison-support filtering removed every observation of some units in an estimated cohort. `_n_g_per_cohort` (N_g, W2025 Eqs. 7.4/7.6) is read off the FINAL sample, and period filtering is the first thing in this estimator that removes SOME units of a RETAINED cohort -- `_filter_sample` keeps every row of every treated unit, and unidentified-cohort exclusion removes whole cohorts. Measured: a cohort supplied with 100 units of which 90 appear only at a dropped period is counted as 10, moving the aggregate from 1.8078 to 3.8157. W2025 Section 7 assumes a balanced panel and does not settle which reading of N_g applies, so the library refuses naming the cohorts and unit counts rather than choosing silently -- mirroring the existing survey + cohort_share refusal. `weights="cell"` never reads N_g; balanced panels drop whole periods and no units, so the all-eventually-treated capability cannot trip the guard. Accuracy of user-facing claims - The zero-cell cohort warning attributed every case to the Section 5.4 no-never-treated normalization. Measured counterexample: cohorts {0, 3, 7} over t=1..5 -- a panel that HAS a never-treated group -- warns on cohort 7, which is zero-cell only because the panel never reaches t=7. Each cause is now explained against the cohorts it actually covers. - The survey refusal points at explicit frame restriction, while REGISTRY and the survey roadmap claimed that restriction "loses nothing -- no PSU or stratum disappears". True on a balanced panel, false in general: measured, restricting a 6-period unbalanced frame to its 5 supported periods removed one PSU and one stratum outright. All three surfaces now state the condition. - CHANGELOG and both LLM guides said covariates on an all-treated panel are categorically rank-deficient, contradicting REGISTRY. Measured: `xtvar` with `demean_covariates=True` (the default) fits cleanly even under `rank_deficient_action="error"`; `exovar`, `xgvar`, and `xtvar` with demeaning off do raise. The guides are the AI-agent contract, so the blanket claim would have steered agents off a supported specification. Coverage - logit and Poisson all-eventually-treated fits and survey refusals, which the filter serves but no test drove. Both assert the cell set, n_obs, groups, warnings and finite inference, and agree with OLS exactly. - `aggregate("group"|"calendar"|"event")` on a filtered fit, where `results.groups` and `time_periods` have both shrunk. - `test_cohort_trends_true_all_treated_panel_is_refused` has asserted a SUCCESSFUL fit since the cell normalization landed; renamed to ..._estimates with a docstring recording the flip. Tracks the unbalanced-panel N_g estimand in TODO.md. * docs: record both pinned Stata SE ratios (G=500 and G=191) The all-eventually-treated arm pins its own measured ratio at G=191 (1.002642), because the gap is cluster-count dependent and the full-panel constant does not transfer. REGISTRY and TODO.md still said only the G=500 ratio was CI-pinned, and both omitted G=191 from the measured ladder -- contradicting METHODOLOGY_REVIEW.md, which already recorded it. Verified the four sibling `G=500` mentions carry a different claim (sup-t bootstrap locking, CCF coverage, the gap's magnitude, a perf benchmark) and correctly need no edit. * docs(tutorial): cite the published Wooldridge (2025) paper in tutorial 16 The Section 5.4 material added here cites Wooldridge (2025) three times, but the bibliography ended at the 2021 SSRN working paper and the 2023 nonlinear paper, and the intro still described the estimator as implementing "Wooldridge's (2021, 2023)" approach. The 2021 SSRN entry is the working-paper version of the same paper, so the reference now leads with the published Empirical Economics citation (matching docs/references.rst) and records SSRN 3906345 / NBER WP 29154 as prior versions, rather than listing what look like two separate works. Also aligns the tutorial's covariate note with the correction made elsewhere in this PR -- default `xtvar` (demean_covariates=True) is full rank, so the rank-deficiency warning names only the specifications it actually applies to -- and adds the all-eventually-treated section to the topics list and key takeaways, which the new cells were missing from.
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
Fixes issue #724 and anchors both staggered estimators to their canonical Stata implementations. Eleven local review rounds surfaced further identification defects in the same code, including a P0; those are fixed here too.
The #724 fix. With
control_group="never_treated"the saturated ETWFE design emitted every cohort×time indicator including the reference period — collinear with the absorbed unit FE — and left generic QR rank detection to remove whichever column it picked. On the realmpdtapanel that silently removedATT(2004, 2004)andATT(2006, 2007), returning 5 of 7 post-treatment cells. The design now omitsref(g)explicitly, per-cohort and weight-aware.The pre-fix survivors were not merely incomplete — they were measured against the wrong baseline.
ATT(2007,2007)computed by hand against references 2006/2005/2004/2003 gives−0.0261 / −0.0571 / −0.0599 / −0.0294; the library returned−0.0571, a correct 2×2 DiD against a silently chosen 2005.Not confined to
never_treated: any cohort whose cell block spans its own cohort dummy hits it, whichanticipation > 0makes reachable on the default path.Stata parity arm. Anchors ETWFE and CallawaySantAnna against
jwdid/csdidon the committedmpdtapanel (pinned SHA-256, so CI needs neither Stata nor network). This replaces a test that validated nothing:test_wooldridge.pyasserted ETWFE == CS within5e-3, false on real data (0.0171 at(2007,2007)), passing only becauseload_mpdta()was silently serving a synthetic effect-homogeneous DGP when its source 404'd (issue #722). Stata's own two implementations reproduce the same disagreement.Identification defects found while reviewing the above:
ATT(g,t)labels. Measured with true effects 1.0 and 3.0:ATT(3,5) = −1.9393(wrong sign) insideoverall_att = +0.7703against a true ~1.8, with no warning underrank_deficient_action="silent". Pre-existing and worse at base.fweight, missing PSU/strata confined to a cohort that exclusion removed produced a confident finite estimate with a warning blaming a missing pre-treatment period. The full design is now validated pre-exclusion.survey_design=+ unidentified-cohort exclusion is refused — deleting rows drops their PSUs/strata from the TSL variance and fromdf_survey = n_PSU − n_strata(measured 22 → 14, finite SE, no diagnostic).rank_deficient_action="error"was silently downgraded to a warning for skipped cells; all three modes now dispatch.Behavior changes (please read before merging)
Deliberate capability regression: all-eventually-treated panels (no never-treated group) now raise. They previously returned finite, silently wrong ATTs — on a true-1.5 DGP:
ATT(3,8) = −0.0096,ATT(5,9) = −0.2393,overall_att = 1.0023. W2025 §5.4 defines the fix (last cohort as control, columns dropped deliberately rather than by QR); the library already applies this to the cohort-trend column but not to the cells. Tracked at High priority — landing it restores these fits.Four existing tests asserted only that such values were finite, which is what let a P0 live in a tested path; they now assert the refusal with the measured numbers in their docstrings.
Ledger rows
M-123/M-124and the CHANGELOG record all user-visible changes.Methodology references (required if estimator / math changes)
jwdid(Rios-Avila) andcsdid— SSC.jwdid—hc1SEs are uniformly belowjwdid's (anti-conservative): ~0.1% at G=500, ~2.8% at G=20. The mechanism is not identified — it trackssqrt(G/(G-1))but sits above it, andsolve_olsalready applies full CR1 — so it is recorded as measured, not diagnosed, and pinned rather than "corrected" with an unverified factor. The never-treated arm shows the same factor over a different cell count, ruling out explanations keyed to the comparison set. CallawaySantAnna shows no such gap.g−1(legitimate per §6.1; documented).TODO.md, with the stopgaps carrying what "done" means.Validation
tests/test_wooldridge.py,tests/test_etwfe_cs_stata_parity.py,tests/test_methodology_wooldridge.py,tests/test_v4_matrix.pycsdid/jwdid/jwdid_never; onlymeta.ssc_versionsmoved (drdidgained a checksum identifier where it previously recorded"unknown").docs/tutorials/16_wooldridge_etwfe.ipynbre-validated withpytest --nbmake. 6992 tests pass across everyWooldridgeDiD/SurveyDesign-touching suite; mypy 2.3.0 clean; black/ruff clean.G=500SE ratio only; the few-cluster ratios quoted in REGISTRY were measured ad hoc and are explicitly labeled as trend-shape rather than gated. The subsample ladder that would pin them is a named required artifact on the SE-derivation TODO row — it is the same instrument that derivation needs.Security / privacy