feat(v4): ETWFE per-period comparison support - restore all-eventually-treated fits (W2025 Sec 5.4) - #730
Merged
Merged
Conversation
…y-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.
…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.
…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.
… 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.
…urate 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.
Overall assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive summary
Methodology
No undocumented estimator, weighting, or variance mismatch found. Code QualityNo findings. PerformanceNo findings. MaintainabilityNo findings. Tech Debt
SecurityNo findings or apparent secrets. Documentation/Tests
Static syntax and JSON validation passed. Runtime tests could not be executed because the review environment lacks pytest and numerical dependencies. |
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.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive summary
Methodology
No undocumented methodology, weighting, identification, or variance mismatch found. Code QualityNo findings. PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo untracked findings. The remaining estimator limitations have actionable SecurityNo findings or apparent secrets. Documentation/Tests
The prior stale G=191 documentation finding is fixed consistently across the registry, backlog, and methodology review. Static Python/JSON validation passed; runtime tests could not run because |
…l 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.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive summary
Methodology
No undocumented methodology, weighting, identification, or variance mismatch found. Code QualityNo findings. PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo untracked findings; remaining limitations have SecurityNo findings or apparent secrets. Documentation/TestsThe previous P2 is resolved by the published Wooldridge (2025) citation at |
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
Restores estimation on all-eventually-treated panels (no never-treated group), which
#729's completeness gate correctly but bluntly refused. Implements the cell half of Wooldridge (2025) Section 5.4 — the library already applied the rule to the cohort-trend column but not to the cohort × time cells, which left the trend-drop branch unreachable dead code.feat— per-period comparison support. A period is retained only if some unit is untreated there; the eligible set is keyed on the regression baseline, not the method. CohortG_maxbecomes the reference and receives no cells, which on a balanced panel is exactly Eq. 5.15.cohort_trends=Trueworks on such panels for the first time, surfacingG-1slopes.test— Eq. 5.15 cell set parameterized over anticipation, reference-move behaviour on both branches, agroups-leak regression guard, nonlinear (logit/Poisson) parity with OLS, and every aggregation surface on a filtered fit.test— new Statajwdid_alltreatedgolden arm as the external anchor.docs— ledger rowM-125, REGISTRY notes, API/tutorial/guide updates, and amendments toM-123/M-124, whose text this change falsified.fix— a cohort-share weighting boundary and three inaccurate user-facing claims (below).Nothing is dropped silently. Every reducing fit reports the periods, the observation count and the branch-correct cause; separately names any cohort left with no cells or stripped of every row; and warns when a filtered period legitimately moves a cohort's reference. Neither warning is gated on
rank_deficient_action. Statajwdidperforms the same reduction and reports only a smallerN— being explicit is a deliberate improvement, not a numerical deviation.Two fail-closed boundaries rather than confident wrong numbers:
survey_design=when rows would actually drop (naive subsetting removes PSUs/strata from the TSL meat and fromdf_survey).aggregate(weights="cohort_share")when filtering removed every observation of some units in an estimated cohort.N_gis read off the final sample, and period filtering is the first thing in this estimator that can remove some units of a retained cohort. 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 applies, so the library refuses rather than choosing silently.weights="cell"is unaffected, and balanced panels drop whole periods and no units, so the restored capability cannot trip the guard.Also corrects three claims that were measurably wrong: the zero-cell warning attributed every case to the Section 5.4 normalization (false on a panel that has never-treated units); REGISTRY and the survey roadmap claimed explicit frame restriction "loses no PSU or stratum" (true only on a balanced panel — measured, an unbalanced frame lost one of each); and CHANGELOG plus both LLM guides called covariates categorically rank-deficient, contradicting REGISTRY — default
xtvarwithdemean_covariates=Trueis full rank.Methodology references (required if estimator / math changes)
WooldridgeDiD(ETWFE) — per-period comparison support; last-cohort normalization; cohort-share aggregation weights.docs/methodology/papers/wooldridge-2025-review.md; contract indocs/methodology/REGISTRY.md→## WooldridgeDiD (ETWFE).jwdidperforms it silently. Estimates are identical; only the diagnostics differ. Documented as a REGISTRY Note.D_{G_max} × Xis NOT normalized. Section 5.4's rule covers four regressor families; this change delivers three (cells, cell × covariate, cohort trend). Supplyingexovar/xgvar, orxtvarwithdemean_covariates=False, therefore stays rank-deficient — coefficients unaffected, butrank_deficient_action="error"raises. Defaultxtvaris full rank. REGISTRY's Section 5.4 Note is narrowed accordingly and the work is tracked inTODO.md.N_gis undefined, so cohort-share aggregation fails closed instead of picking a reading (see Summary). Tracked inTODO.md.jwdidparity evidence are balanced-panel-only; the general rule is the support predicate. No unbalancedjwdidarm exists yet (tracked).Validation
tests/test_wooldridge.py(newTestComparisonSupportFilteringplus re-pinned audit sites),tests/test_methodology_wooldridge.py,tests/test_etwfe_cs_stata_parity.py(newTestAllEventuallyTreatedVsStataJwdid),tests/test_v4_matrix.py.jwdidon the pinnedmpdtapanel with never-treated counties dropped — identical cell set{(2004,2004), (2004,2005), (2004,2006), (2006,2006)}, identicalN(764 of 955), point estimates agreeing to ~1e-15. SEs are pinned as a freshly measured ratio at this arm's cluster count (1.002642 at G=191); thehc1gap is cluster-count dependent and remains open, so the sibling arms' G=500 constant does not transfer.{3, 5, 8}overt = 1..9with a true ATT of 1.5 → cells(3, 3..7)and(5, 5..7),overall_att = 1.5502, cohort 8 receiving none. Estimates are the absolutetau_gt, not a relative contrast.docs/tutorials/16_wooldridge_etwfe.ipynbcovers the restored fit and the drop warning; executed under--nbmake.mypyclean;black/ruffclean.Security / privacy