Conversation
Generator + committed goldens for the upcoming `attgt_weights` / `decompose_twfe_weights` surface, ported from Brantly Callaway's `twfeweights` R package (MIT). Three fixtures: `mpdta` (real; non-1..T time labels), `sim_staggered` (equal cohorts, real pre-trend so pretrend_bias != 0), and `unbalanced_cohorts` (120/70/60 — breaks the p_g == 1/3 degeneracy that would let a cohort-share bug pass silently on the equal-cohort fixture). Pins `twfe_weights`/`attO_weights`/`att_simple_weights`, `implicit_twfe_weights` (no-cov, covariate, gmin1), `implicit_aipw_weights`, `twfe_cov_bal`/`aipw_cov_bal` + the summary roll-up, and the two two-period kernels that will have no public Python surface. The no-covariate decomposition is generated with a TIME-INVARIANT covariate rather than `xformula = ~1`: upstream builds an nT x 0 model matrix on that branch and `fixest::demean` segfaults on a zero-column matrix (reproduced in isolation, fixest 0.14.2 / R 4.6.1). Double-demeaning annihilates a time-invariant regressor exactly, so the call is numerically the `~1` branch — and the Python test will assert both `covariates=None` and `covariates=[<col>]` against this one golden, proving the equivalence rather than assuming it. R is only needed to regenerate the JSON, never to run the tests. Co-Authored-By: Claude <noreply@anthropic.com>
…iners Result containers for the incoming TWFE implicit-weight diagnostics, landed ahead of the compute module so they pin the output schema and the Diagnostic contract before any math depends on them. Both subclass Diagnostic: they assess what a regression implicitly weights rather than estimating an effect, so neither carries the estimator quintet. The headline scalars are deliberately named `implied_att` and `estimate` rather than `att` so they do not read as inference-bearing. Output columns are diff-diff's (`group`, `time`, `post`, `weight`, `att`), not R's (`time.period`, `attgt`). Covariate balance is a result-object method rather than a mutate-in-place second pass as in R: `covariate_balance(level="summary"|"cell")` reads a table computed at construction time, so the result never retains the raw panel. It raises with the fix inlined when balance was not requested. Names are clearly separated from the existing dCDH surface (`twowayfeweights` / `TWFEWeightsResult`), which weights (unit, time) cells; these weight ATT(g,t) parameters. Roster (M-091) and the shared construction fixture updated; the roster test auto-enrolls both classes. Co-Authored-By: Claude <noreply@anthropic.com>
One entry point folding R's three separate weight functions behind
`aggregation=`: "twfe" (twfe_weights), "overall" (attO_weights, ATT^O), and
"simple" (att_simple_weights, ATT^simple). Reports what each estimand
implicitly puts on every group-time effect, plus the negative-weight share
that makes the staggered-TWFE pathology legible.
Takes a fitted CallawaySantAnnaResults as the primary input, reading cohort
masses off the aggregation bookkeeping so no raw panel is needed; a
(gt_frame, data=, unit=, time=, first_treat=) fallback consumes
`result.to_dataframe("group_time")` verbatim.
Design restrictions are hard errors, not warnings, each naming its fix:
aggregation="twfe" needs base_period="universal" and
control_group="never_treated" (matching R's own stop()s), and no aggregation
accepts a repeated-cross-section or unbalanced-fallback fit, whose cohort
shares are not comparable across periods.
Deviation from R: cohorts and periods are mapped to positional time before
the (maxT - g + 1)/T arithmetic. R evaluates that on raw labels, which is
only correct on consecutive integers; positional time is bit-identical there
(mpdta 2003..2007 -> 1..5 both give 4/5 at g=2004) and correct on gapped
grids. Pinned by a test that remaps periods to 10,20,30,40,50.
R's keep_untreated= is not exposed: it synthesizes G=0 rows that are
excluded from every normalization and contribute exactly zero.
Parity: machine precision (max |dw| = 4.7e-16) against R twfeweights 0.9.0
on 3 fixtures x 3 aggregations. Primary assertions feed R's own ATT(g,t)
back in, isolating this module from CallawaySantAnna-vs-`did` parity; a
separate, deliberately looser class covers the composed end-to-end path.
Also fixes a generator bug: R stores `post` as a FACTOR, so as.integer()
emitted level codes 1/2 rather than 0/1.
Known-red until the docs commit: test_doc_deps_integrity.py wants a
docs/doc-deps.yaml entry, which lands with the API page.
Co-Authored-By: Claude <noreply@anthropic.com>
…ance
Re-derives a TWFE estimate from its ATT(g,t) building blocks, reporting the
implicit weight on each cell, the contribution of PRE-treatment cells
(`pretrend_bias` - parallel-trends violations rather than treatment), and
implicit-weight covariate balance via `result.covariate_balance()`.
Takes the raw panel rather than a fitted CS result because it re-estimates:
it double-demeans treatment and covariates and forms its own group-time
contrasts, so there is no ATT(g,t) table it could consume, and a CS result
carries no panel by design. The two surfaces are tied by an identity that
the suite pins:
attgt_weights(cs, aggregation="twfe").implied_att
== decompose_twfe_weights(panel, ...).estimate
Two numerical points, both found by disagreeing with the goldens and then
proving which side was right:
1. Covariates that double-demeaning ANNIHILATES are now dropped before the
projection, judged against each column's own PRE-demeaning norm. A
time-invariant regressor leaves a column of pure rounding noise (~1e-16
against a raw scale of ~1); regressing on it amplifies that by ~1e16 and
silently corrupted the per-cell weights. A rank test on the demeaned
matrix alone cannot see this - there, 1e-16 is simply the largest pivot.
With the fix, covariates=None and covariates=[<time-invariant col>] agree
to 1e-15 on every fixture, which is exactly the equivalence the
no-covariate golden relies on.
2. Cells whose comparison-group implicit weights are constant AND average to
zero make `resid / mean(resid)` a 0/0. On sim_staggered (equal cohorts at
g in {0,3,4}, T=5) this happens exactly at t=3, where
-E_3[D] + mean_t E_t[D] = -1/3 + 1/3. We take the limit (a constant over
its own mean is one); R divides the rounding errors and lands ~3e-4 away.
Verified against a hand-computed contrast that needs none of this module:
ours is exact to 4.4e-16. The weights on such cells cancel exactly in the
aggregate, so `estimate` is unaffected - the suite gates `estimate`
tightly on every fixture and relaxes only the per-cell and
decomposition/remainder-split assertions, on cells DETECTED as degenerate
rather than on a hard-coded fixture.
Parity vs R twfeweights 0.9.0: estimate and per-cell weights at machine
precision on all 3 fixtures x 4 configurations; all 11 balance statistics at
machine precision, including `frac_treated_extreme`, which required
reproducing BMisc's weighted-ECDF plus `stats:::quantile.ecdf`'s
pseudo-sample reconstruction rather than a plain quantile.
method="aipw" is not implemented yet and raises listing the accepted values.
Co-Authored-By: Claude <noreply@anthropic.com>
Plotting (replacing upstream's ggtwfeweights S3 methods) and every documentation surface the new API owes. plot_twfe_weights(result, kind="auto"|"weights"|"balance") lives beside plot_bacon in visualization/_diagnostic.py and dispatches on either result type. The weights view puts weight on x and ATT(g,t) on y with zero lines, so negative-weight cells sit visibly left of the axis; the balance view plots unweighted against implicitly-weighted covariate differences with a no-improvement diagonal. "auto" picks balance when a balance table is present. Docs: a REGISTRY.md section carrying the weight equations, the cross-surface identity, the tolerance table with per-gate rationale, and eleven explicit Note/Deviation-from-R entries - including the fixest zero-column segfault and its root cause, the annihilated-covariate drop, and the 0/0-cell limit, so the two places we deliberately differ from R are recorded rather than discovered later by a reviewer. Two paragraphs separate this surface from `twowayfeweights` (dCDH, weights (unit, time) cells) and from BaconDecomposition (decomposes into 2x2 comparisons), since all three are "TWFE weight" diagnostics and the distinction is the thing a reader most needs. Also: docs/api/twfe_weights.rst with runnable examples, four api/index.rst registrations (2 result classes, the plot, 2 functions, toctree), doc-deps.yaml group + sources entries, a README one-liner in Diagnostics & Sensitivity, llms.txt catalog entry, llms-full.txt API + result blocks, a references.rst sub-entry naming the upstream package and its MIT copyright, and a changelog.d fragment. This closes the doc-deps gate the attgt_weights commit left red. Verified: 14378 tests collect clean; docs IA, doc-deps integrity, diagnostic roster, guides, changelog-fragment, serialization and all visualization suites green (903 passed, 43 skipped). Co-Authored-By: Claude <noreply@anthropic.com>
…arity gates Addresses all 21 items in igerber's review of #812. The port's R-parity was accepted; almost everything here is about the paths that go BEYOND the R reference (the ATT(g,t)-frame input, user weights, non-standard cohort labels), where R never faced the input so the port had no validation. Correctness (items 1-5), all reproduced by the reviewer: - Cohort labels: never-treated is exactly 0 or +inf. NaN / -inf raise instead of being silently absorbed into cohort 0 (a single NaN label moved `estimate` by ~1.4% with no warning). Within-unit invariance now uses nunique(dropna=False) at all three invariance sites; non-finite period labels are rejected up front. - Balance roll-up propagates NaN as R does: `_frac_treated_extreme` returns NA for a covariate with <3 distinct values, and the summary no longer turns that into 0.0 via pandas' NaN-skipping sum. Masks on the `post` column, not on a zero roll-up weight (a zero-weight post cell still contributes). - `aggregation="twfe"` now enforces R's third restriction (xformla == ~1): fits record their covariate names on the aggregation kit, at both build sites (staggered.py and dml_did.py), and a covariate-adjusted fit raises. A kit predating the bookkeeping warns; a non-CS result is a TypeError. - ATT(g,t)-frame input: duplicate cells, non-finite group/time labels and non-finite effects are rejected, and an incomplete grid fails closed for all three aggregations. Two structural gaps mirror R instead of raising: a cohort with no estimable post cell is dropped (did's first-period drop), and under control_group="not_yet_treated" the CS estimands average over each cohort's available post periods (aggte). Both warn. - Unit weights must be finite, non-negative, with positive total and treated mass; never-treated mass is required only where the comparison group enters the formula, so overall/simple still work without one. Parity and house conventions (items 6, 7, 9): - Parity now asserts pretrend_bias, post_only, effective_sample_size and cell ess/remainder. At the documented 0/0 cells the expectation is rebuilt from R's OWN cells with our limit substituted only where R's number is noise, so the assertion stays anchored to R. The scalar-split relaxation is restricted to fwl_gmin1, and fwl_gmin1 cells are asserted. - The FWL linear algebra goes through the house helpers: within_transform for the two-way demeaning and solve_ols for the weighted solve. The bespoke pivoted QR is gone - it was the same norm-pivoted QR solve_ols uses, so its "drops later columns first" docstring was inaccurate and nothing R-specific was lost. A frozen-numbers pin captured on the pre-refactor code guards the weighted branches, which no parity fixture covers. - negative_weight_share counted pre cells, so it read ~0.5 in every staggered design. n_negative_post / negative_post_weight_share report the actual pathology; summary() leads with them. Tests (items 8, 13, 18, 19): decomposition edge cases, both plot backends, collinear covariates, and regression tests for items 1-5. plot_twfe_weights gains the backend="plotly" split its file-mates have. The constructed fixtures are derived from their cells instead of carrying stale literals, and the two weak tests now assert hand-computed values and all five exported names. Goldens (items 10-12): regeneration is numerically inert - every one of the 192 changed numbers is a group/time label, none elsewhere. Cells now carry original period labels throughout (implicit_* run in positional time), and the tests assert labels rather than array position. The duplicated mpdta panel CSV is dropped in favour of the shared mpdta_stata_panel.csv plus a derived-column expression, with the generator asserting the two agree. The AIPW blocks are labelled reserved for the follow-up rather than left looking unused. Docs (items 14-17, 20, 21): README back to the sibling shape, changelog trimmed, REGISTRY rewritten (the new hard errors, both R-mirroring carve-outs, the 0/0 magnitudes enumerated, the annihilation note recast as numerical hygiene with its threshold limitation stated), llms-full contract prose and a plot subsection, doc-deps banner, requirements.R (twfeweights is not on CRAN), and twfe_weights.rst registered in the snippet harness after fixing its first_treat column name and making each block self-contained. The weighted ECDF is O(n log n) instead of quadratic, holding parity at 1e-9. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses igerber's second review of #812 (1 rename decision + items 1-18), rebased onto current main. As before, most of it is about the paths R never had to handle: the ATT(g,t) frame, anticipation, unbalanced panels, and non-standard period labels. Decision - rename `pretrend_bias` to `pre_period_contribution`. The field is the SAMPLE contribution of the pre-treatment cells, which can reflect differential pre-trends OR sampling variation (the diagnostic carries no inference), so it is diagnostic evidence about the earlier-period restrictions, not proof the identifying assumption fails post-treatment. The three doc surfaces say so. Correctness (items 1-6), all reproduced by the reviewer: - One canonical numeric time key. Period labels are coerced once at validation and that key drives sorting, reshaping, cohort mapping and the grid; using the raw column lets "10" sort before "2" and silently rebuilds a different panel. Numeric and numeric-string labels now decompose identically ([1,2,10,11,12] pinned). - `anticipation` is honoured on `"overall"`/`"simple"`: post cells become `t >= g - anticipation` and the window enters the required-cell grid, the available-period divisors and the `post` column. The fit is read off the aggregation kit; the frame path takes an explicit `anticipation=` and the fitted path rejects the kwarg instead of ignoring it. `"twfe"` deliberately keeps `t >= g` (the regression's own indicator does not anticipate, and R's twfe_weights has no anticipation argument). Each CS estimand is pinned against `results.aggregate(...)` with anticipation=1 under both control groups. - Unbalanced panels are rejected: the kit records `is_balanced`, and the frame path requires exactly one observation per unit-period. The cohort shares and E_t[D] assume a fixed unit set. - Non-finite outcomes, regression covariates and balance covariates fail closed instead of returning an all-NaN decomposition. - The zero_treated_control carve-out keys on the `skip_reason` VALUE, not on `control_group`, so the fitted and frame paths agree (the frame path has no control_group to read). A not-yet-treated fit is added to the fit-vs-frame agreement test. - A cohort is dropped only when the drop is structural: treated in the first observed period, or every missing post cell carries zero_treated_control. A mid cohort blanked out any other way (NaN effects, skip_reason None) raises instead of silently leaving the estimand. Numerics and plotting (items 8-11): - The annihilation threshold is the accumulated rounding-noise scale (`sqrt(n_obs) * 64 * eps * max(raw, 1)`) instead of a fixed 1e-10, which was five orders too loose (it discarded a 1e6-level covariate with genuine within-sd 1e-4) while still annihilating mpdta's lpop. A dropped covariate no longer appears in `result.covariates`. - The weights-view title counts POST-only negatives (a healthy panel was titled "(5 negative)"); the signed balance view draws the no-improvement diagonal over [-limit, limit] in both backends; the plotly renderer's dead `text`/`customdata` lines are gone while `text` still feeds the hover. - A wrong-length `weights=` now fails with both lengths before the excluded-cohort slice, instead of a raw IndexError. Docs (items 7, 16, 17): REGISTRY corrects R's mechanism for the second carve-out (`pre_process_did` truncating to t < max(g) and recoding the last cohort, not aggte averaging) and records the new guarantees; the R generator header, the mpdta check comment and `meta.mpdta_provenance` now say identical-on-integers / 1e-14-on-floats rather than "bit-for-bit"; the `plot_twfe_weights` subsection moves out of the estimator catalog to sit beside plot_bacon under Visualization. Tests (items 12-15): degenerate-cell ESS is asserted at the analytic `n_control`; a two-covariate frozen-numbers pin covers the multi-column solve_ols branch for the first time; the re-implementation test that could not fail is deleted and the export test covers all five names; parity asserts `n_negative_post` / `negative_post_weight_share` against R's own golden weights. New regression tests cover items 1-6, 10 and 11. Verified: the two twfe suites at 181 passed; docs IA, doc-deps, guides, changelog fragments, diagnostic roster, serialization, aggregate-contract, spillover, staggered, dml_did and all visualization suites green; full run 14793 passed. ruff/black clean, mypy at zero errors.
Rename the estimand selector to the house aggregate(type=) vocabulary and the result field to .level (values unchanged: "twfe" | "overall" | "simple"). Declare the accepted values as ATTGTWeightsResult.LEVELS (ClassVar) and source the module's _TYPES from it. The private helper's parameter becomes `estimand` because its body needs the builtin type(). Naming-guard entries for the old keyword are removed (no hit left to explain); the surviving [time] entries keep a note on the level/M-044 coexistence. The feature is unreleased, so no deprecation alias or ledger row.
… rule, producer-recorded completeness
attgt_weights applied the anticipation window to POSITIONAL indices while
CallawaySantAnna applies it to raw period labels, so on a gapped calendar
(10, 20, 30, 40; anticipation=1) it flagged (20,10) as post and returned
1.5230 against aggregate("simple") 2.2844. The window now counts raw time
units on the post mask, the required grid, the available-cell divisors and
the structural rule; "twfe" keeps t >= g for its own post window.
The whole-cohort drop and the incomplete-grid carve-out are now keyed on the
estimator's own skip_reason set {missing_period, zero_treated_control,
zero_weight_mass} (the cells aggregate() finite-masks) instead of
zero_treated_control alone plus a positional first-period shortcut, so a
cohort CS could not estimate at all under the window is dropped for every
type (as did::pre_process_did drops it) and a cohort kept through a single
estimable cell is averaged over what it has (as aggregate() keeps it; a
documented deviation from R's drop under base_period="varying"). A cohort
blanked without a reason still fails closed under any window. Bare frames
keep only the first-period shortcut, in raw time.
Panel completeness on the aggregation kit is now the producer's own record:
every outcome cell finite AND no unit dropped by the estimator's per-cell
complete-case mask (DMLDiD sets complete_case_drops from the units its cell
loops excluded; CS's NaN-covariate fallback keeps masses intact). Kits
without the record or the covariates record (pickled by <= 3.12.0) fail
closed with a refit message instead of warn-then-continue.
decompose_twfe_weights reports the ORIGINAL time labels on every surface
(cells, summary, balance rows, plots); the numeric key stays internal, a
column mixing two spellings of one period is rejected, and the frame path
detects duplicate cells on the numeric keys so "3" and 3.0 cannot be
double-counted.
Docs: REGISTRY equations in label form, the structural-rule / completeness
/ anticipation / canonical-key Notes rewritten (R did 2.5.1 pre_process_did
provenance stamped), identity conditions tightened, guide and changelog
updated. Tests cover the structural drop for all three types, the
varying-base keep, reason-less blanks under wide windows, gapped calendars
against aggregate(), zero_weight_mass, the bare-frame shortcut, DML
complete-case drops (incl. the bad-control lane), CS inf outcomes, the
legacy-kit refusal for both producers, label round-trips, and the weighted
identity with the same survey weights on both sides.
…cking rows - "pre-trend contribution" -> "pre-period contribution" on README, the API page, the module docstring and the REGISTRY intro (the term is a sample contribution that can be pre-trends or sampling variation) - API-page examples print n_negative_post: the post-only counts are the pathology, the all-cell counts are structural - R generator header pins twfeweights == 0.9.0 (matches the stopifnot) and states the fixture cross-check precision honestly - TODO rows for decompose_twfe_weights(method="aipw") and the tutorial section (both follow-on PRs from #812) - multi-covariate pin gains a brute-force OLS anchor
…d on zero-mass cohorts and phantom cohorts
attgt_weights(fitted_result, weights=w) recomputed only the weight vector
under w and multiplied it into ATT(g,t) estimated under the fit's own
weights, so implied_att was neither the weighted nor the unweighted
estimand (1.4349 vs the weighted fit's 1.4222 on the frozen weighted
fixture). The fitted path now rejects an explicit weights= and uses the
fit's SurveyDesign weights; the DataFrame path keeps weights= under the
documented contract that the frame's ATT(g,t) were estimated under them.
The weighted pins are re-captured from a natively weighted fit and anchored
to decompose_twfe_weights(weights=).estimate and aggregate("simple").
decompose_twfe_weights rejects a treated cohort whose units all carry zero
sampling weight by name (its cells were 0/0 and poisoned the decomposition
with NaN silently). attgt_weights' frame path rejects a cohort in the
ATT(g,t) frame that no unit in data= belongs to (was a raw KeyError), and
keys its skip_reason lookups on canonical labels so a numeric-string frame
keeps the structural carve-out (a string-keyed skip was searched as an int
and wrongly rejected).
effective_sample_size is R's verbatim (sampling weights cancel); documented
as a REGISTRY Note rather than changed, since the per-cell ess is R-gated.
The per-cohort share in the FWL loop is computed once per cohort instead of
once per cell. Class-scoped instance-method fixtures moved to module level
(pytest 10 removal). REGISTRY legacy-kit sentence aligned with the code.
Overall assessmentExecutive summary
MethodologyFinding 1: Ambiguous per-unit weight alignment
Documented deviations
Code QualityNo additional findings. PerformanceNo findings. MaintainabilityNo findings. Tech Debt
SecurityNo findings or apparent secrets/PII. Documentation/TestsNo separate finding. The changelog fragment follows the project convention and Full pytest execution was unavailable because Path to Approval
|
This branch has not been deployed
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
twfeweightsR package -attgt_weights,decompose_twfe_weights,plot_twfe_weights, the twoDiagnosticresult containers, R-golden parity fixtures, docs and REGISTRY section.refactor(twfeweights):attgt_weights(type=)andATTGTWeightsResult.level(values unchanged:"twfe" | "overall" | "simple", declared asATTGTWeightsResult.LEVELS); naming-guard entries for the old keyword removed.fix(twfeweights): anticipation window applied in raw time units as CallawaySantAnna applies it (positional was wrong on gapped calendars); the whole-cohort drop and the incomplete-grid carve-out keyed on the estimator's ownskip_reasonset (missing_period/zero_treated_control/zero_weight_mass) for everytype; panel completeness on the aggregation kit recorded from the producer's own complete-case decisions (DMLDiD setscomplete_case_drops), with legacy kits (<= 3.12.0) refused; decomposition reports the original time labels; frame-path duplicate cells detected on numeric keys.fix(twfeweights): the fitted path rejects an explicitweights=(its ATT(g,t) were estimated under the fit's own survey weights; a different vector madeimplied_atta hybrid); zero-mass treated cohorts and phantom frame cohorts fail closed by name; canonical skip-reason keys;effective_sample_sizedocumented as R's verbatim (sampling weights cancel).docs(twfeweights): pre-period-contribution wording, post-only pathology in the examples, R generator header, TODO rows for the AIPW method and the tutorial section (follow-on PRs).Methodology references (required if estimator / math changes)
attgt_weights), FWL decomposition (decompose_twfe_weights)twfeweights0.9.0 (MIT)docs/methodology/REGISTRY.md"TWFE Weight Diagnostics" - positional time rescaling, 0/0 cells report the limit, consolidated API,keep_untreatednot exposed, a varying-base cohort kept through one estimable cell (follows diff-diff'saggregate(), not R'spre_process_diddrop), post-lasso and AIPW out of scope,effective_sample_sizepreserved verbatim.Validation
tests/test_twfe_weights.py,tests/test_twfe_weights_parity.py(R goldens untouched),tests/test_naming_guard.py,tests/helpers/results_foundation.pyresults.aggregate()("simple"exactly,"overall"against the cohort-share combination ofaggregate("group")); the weighted"twfe"identity holds to 4e-16 againstdecompose_twfe_weights(weights=); Rdid2.5.1pre_process_didsource verified for the first-period / truncation claimsSecurity / privacy
Changelog
changelog.d/20260831-twfe-weight-diagnostics.md)