Skip to content

fix(staggered): CS panel reg/ipw influence functions + per-cell SEs to DRDID parity#619

Merged
igerber merged 3 commits into
mainfrom
fix/cs-reg-ipw-if-se
Jul 5, 2026
Merged

fix(staggered): CS panel reg/ipw influence functions + per-cell SEs to DRDID parity#619
igerber merged 3 commits into
mainfrom
fix/cs-reg-ipw-if-se

Conversation

@igerber

@igerber igerber commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fixes two CallawaySantAnna panel variance defects (point estimates unchanged - the omitted terms are mean-zero, so ATTs always matched R at ~1e-11 while SEs drifted):
    1. reg + covariates: the per-cell influence function omitted DRDID::reg_did_panel's OLS estimation-effect term (asy.lin.rep.ols %*% M1). Per-cell SEs sat 4-13% and aggregated (simple/group/event-study) SEs 3-20% from the R golden fixtures, anti-conservative on some designs (0.958x on the two-period golden). The projection is evaluated in the centered basis 1/n_c + (x - x̄_c)'G^{-1}(x̄_t - x̄_c) - algebraically identical to R's raw-Gram form but offset-invariant (the raw Gram squares the design conditioning; a large constant covariate offset would push the equilibrated rank check below threshold and silently truncate a genuine direction). The bread is hoisted per control group on the vectorized path; the intercept-only collapse is exactly the old -resid_c/n_c.
    2. ipw + covariates (unweighted branch): the per-cell SE used a weighted population variance never scaled by an effective sample size (~7x inflated, uniformly 6.8-7.8x across the golden cells), and the IF lacked std_ipw_did_panel's propensity-score estimation-effect correction (asy.lin.rep.ps %*% M2; aggregated SEs ~2.4% off). The survey ipw branch already carried both (Phase 7a); the fix mirrors it with the correction fused as (D - ps) * (X @ (H^{-1} M2)).
  • All panel reg/ipw per-cell SEs (including no-covariate, where ipw's var_c*(1-p)/(n_c*p) plug-in equaled R only at treated-share 0.5) now derive from the same IF that feeds aggregation: sqrt(sum(phi^2)), the convention dr always used. Downstream consumers inherit: t/p/CI, aggregated SEs, event_study_vcov (HonestDiD input), bare-cluster= CR1 per-cell override, multiplier-bootstrap SEs.
  • Siblings already correct and untouched: RCS (_outcome_regression_rc/_ipw_estimation_rc carry the corrections), DR everywhere, survey-ipw branch, triple-diff (own code), ContinuousDiD (continuous_did.py:995-1191 is a DRDID-validated twin of the same algebra).

Methodology references (required if estimator / math changes)

  • Method name(s): Callaway-Sant'Anna staggered DiD; outcome-regression and standardized-IPW panel estimators
  • Paper / source link(s): Callaway & Sant'Anna (2021, J. Econometrics); Sant'Anna & Zhao (2020, J. Econometrics) - DRDID::reg_did_panel / DRDID::std_ipw_did_panel (DRDID 1.3.0)
  • Any intentional deviations from the source (and why): propensity scores clipped at pscore_trim rather than R's drop-trimming at trim.level=0.995 (pre-existing; differs only at extreme propensities); no-covariate ipw treated as unconditional (R's intercept-only logit has an identically-zero estimation effect in the IF, so values match); DR no-covariate per-cell SE keeps its ddof=1 plug-in (O(1/n) from R; TODO row). All documented in docs/methodology/REGISTRY.md (CallawaySantAnna standard-error notes).

Validation

  • Tests added/updated: tests/test_methodology_callaway.py (R-free numpy reconstructions of both DRDID influence functions: unweighted, survey-weighted, not_yet_treated per-pair-bread, and golden-cell variants - written first and failing on the unfixed tree with the known gaps), tests/test_csdid_ported.py (previously-unasserted golden per-cell + aggregated reg/ipw SE blocks enabled at 1e-8/1e-6, observed ~5e-12; fresh R did 2.5.1 ipw aggregation yardsticks with provenance), tests/test_staggered.py (rank-guard warning + rank-0 NaN-bread tests parametrized over all methods; new behavioral tests: ps-fallback equivalence, underdetermined cells, universal base period + anticipation, uniform-weights producer equivalence), stale docstrings updated in tests/test_se_accuracy.py / tests/test_survey_r_crossvalidation.py; benchmarks/R/generate_csdid_test_values.R extended with the ipw aggte blocks for the next fixture regeneration (JSON not regenerated here).
  • Backtest / simulation / notebook evidence (if applicable): perf guard vs b56931a6 at 100k units x 20 periods (5-rep medians): cov5/cov20 reg +2.8%/+4.6%, cov20 ipw +2.8%, cov5 ipw +9.5% (+69ms; the per-cell PS Hessian the correction requires - R pays the same cost). ATT |delta| = 0.0 in all 8 arms; memory flat. Full suite: 8358 passed; the single test_survey_dcdh bit-identity failure under full-suite ordering reproduces byte-identically on pristine origin/main (pre-existing backend-arm mis-detection; TODO row filed). Tutorials 02/16 contain no reg/ipw+covariate cells, so no notebook re-execution is needed.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

🤖 Generated with Claude Code

https://claude.ai/code/session_01X4AzrFUMqJxcUumSH31mSr

…o DRDID parity

Two defects fixed (point estimates unchanged - the omitted terms are
mean-zero, so ATTs always matched R at ~1e-11 while SEs drifted):

1. reg+covariates IF omitted DRDID::reg_did_panel's OLS estimation-effect
   term (asy.lin.rep.ols %*% M1): per-cell SEs 4-13% and aggregated SEs
   3-20% from R, anti-conservative on some designs. The projection is
   evaluated in the centered basis (1/n_c + (x-xbar_c)'G^{-1}(xbar_t-xbar_c)),
   offset-invariant by construction, with the bread hoisted per control
   group on the vectorized path.
2. unweighted ipw+covariates per-cell SE used a weighted POPULATION
   variance never scaled by an effective sample size (~7x inflated), and
   the IF lacked std_ipw_did_panel's PS estimation-effect correction
   (aggregated ~2.4% off). Mirrors the already-correct survey branch,
   with the correction fused as (D-ps)*(X @ (H^-1 M2)).

All panel reg/ipw per-cell SEs (incl. no-covariate) now derive from the
same IF that feeds aggregation: sqrt(sum(phi^2)) (dr convention).
Downstream consumers inherit: t/p/CI, aggregated SEs, event_study_vcov,
cluster-override CR1, multiplier bootstrap.

Validation: R-free numpy reconstructions of both DRDID IFs (incl. survey,
not_yet_treated, golden-cell variants); golden per-cell + aggregated
reg/ipw SE assertions enabled (~5e-12 observed); fresh R did 2.5.1 ipw
aggregation yardsticks (~1e-10); behavioral tests for NaN-bread,
underdetermined cells, ps fallback, universal base period, rank-guard
warning (now fires for reg/ipw), uniform-weights producer equivalence.
Perf guard at 100k units x 20 periods: reg within noise (+2.8/4.6%),
ipw +2.8-9.5% (intrinsic per-cell PS Hessian of the new term).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4AzrFUMqJxcUumSH31mSr
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Overall Assessment

⚠️ Needs changes — one unmitigated P1 finding.

Executive Summary

  • Affected methods: CallawaySantAnna panel reg and ipw SE / influence-function paths.
  • Main reg/IPW DRDID parity direction matches the registry: reg adds the OLS estimation-effect term; IPW adds the PS score correction.
  • P1: reg+covariates mishandles rank-0 centered covariate bread, producing NaN SEs for valid intercept-only-equivalent cells.
  • No inline inference anti-patterns found in the changed paths; inference is routed through safe_inference.
  • I could not run tests locally because the review environment lacks numpy.

Methodology

Finding 1 — P1: Centered reg bread rank-0 incorrectly NaN-poisons valid SEs

Location: diff_diff/staggered.py:L1437-L1443, diff_diff/staggered.py:L1605-L1615, diff_diff/staggered.py:L2703-L2712

Impact: The new centered-basis projection for CallawaySantAnna(estimation_method="reg", covariates=...) calls _safe_inv() on the centered control covariate Gram. If the supplied covariates have zero within-control variation in a cell, e.g. covariates=["xc"] where xc is constant, that centered Gram has rank 0. _safe_inv() returns an all-NaN inverse for rank 0, so proj_c = 1/n_c + Xc_centered @ (bread @ d_tc) becomes NaN even though the intercept term alone identifies the collapsed projection. The ATT remains finite, but per-cell IF/SE and aggregated SE become NaN. This violates the registry’s documented “finite SE on the identified subset” contract for rank-reduced covariates.

The DRDID source supports the intended methodology: panel OR uses the OLS estimation-effect term and IF-based SE; panel standardized IPW uses Hajek weights plus the PS score/Hessian correction and IF-based SE. The issue here is not the high-level DRDID correction, but the rank-0 handling introduced by the centered implementation. (raw.githubusercontent.com)

Concrete fix: Add a centered-bread helper for reg OR IFs: if the centered covariate Gram has rank 0, use a zero matrix for the centered correction and keep proj_c = 1/sum(W_c). Preserve the rank-guard warning/tracker. Apply this in both the vectorized unweighted path and the general/survey _outcome_regression path.

Code Quality

No additional code-quality blockers found. The fused IPW correction and batch inference usage are coherent.

Performance

No findings. Bread hoisting and fused IPW correction are reasonable implementation choices, not methodology issues.

Maintainability

No findings beyond the P1 above. A small helper for centered OR bread would reduce drift between the vectorized and survey/general reg paths.

Tech Debt

P3: Residual deferred items are tracked

Location: TODO.md:L35-L37, docs/methodology/REGISTRY.md:L519-L523

Impact: DR no-covariate per-cell SE parity and IPW fixture regeneration are explicitly tracked/documented.

Concrete fix: No action required for this PR.

Security

No findings. I did not see secrets or security-sensitive changes.

Documentation / Tests

P1: Missing regression test for constant-only reg covariates

Location: tests/test_staggered.py:L1583-L1611, tests/test_staggered.py:L1658-L1664

Impact: Existing tests cover ["x1", "xc"], where the centered Gram still has rank from x1. They do not cover ["xc"] alone, where the new centered bread becomes rank 0 and exposes the NaN SE bug. The test comment says rank-0 is unreachable because of the intercept, but the new centered covariate-only Gram makes it reachable.

Concrete fix: Add tests with generate_staggered_data_with_covariates(seed=789), set xc = 5.0, fit reg with covariates=["xc"], and assert effects/SEs match the no-covariate reg fit with finite overall SE. Add the same check with SurveyDesign(weights="weight").

Path to Approval

  1. Fix centered reg OR bread handling so rank-0 centered covariate variation uses zero centered correction, not an all-NaN inverse.
  2. Add unweighted reg test for constant-only covariate ["xc"] matching the no-covariate fit.
  3. Add survey-weighted reg test for constant-only covariate ["xc"] matching the no-covariate survey fit.

…n, not NaN

CI review P1: with a constant as the ONLY reg covariate the centered
estimation-effect Gram is rank-0, and _safe_inv's all-NaN sentinel
NaN-poisoned per-cell/aggregated SEs for cells whose intercept-only
projection is fully identified (pre-fix raw-Gram column-drop and the
no-covariate fit both give finite SEs). New _centered_or_bread helper
(shared by all three reg sites) maps the rank-0 sentinel to a zero
correction - proj collapses to 1/sum(W_c), equal to the no-covariate
fit - while partial deficiency and the aggregate rank-guard warning
are unchanged. ipw/dr breads are over [1, X] and keep NaN propagation
(true pathology); the rank-0 monkeypatch test is re-scoped to ipw/dr
with constant-only reg equivalence tests (unweighted + survey) added.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4AzrFUMqJxcUumSH31mSr
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 9cb5f26a2a67cdd82c3cd4121efd90d6ce65ede5


Overall Assessment

Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • Affected methods: CallawaySantAnna panel reg and ipw influence-function / per-cell SE paths.
  • The prior P1 is resolved: centered rank-0 reg bread now collapses to a zero centered correction, with unweighted and survey regression tests.
  • The main methodology change matches the documented registry contract and DRDID source direction: reg carries the OLS estimation-effect term, IPW carries the PS score correction, and per-cell SEs come from sqrt(sum(phi^2)).
  • No new inline inference anti-patterns found; modified paths use safe_inference / safe_inference_batch.
  • I could not execute tests locally: this environment lacks numpy and pytest.

Methodology

P3 — Prior P1 resolved, no action required

Location: diff_diff/staggered.py:L2613-L2633, diff_diff/staggered.py:L1593-L1609, diff_diff/staggered.py:L2719-L2725, tests/test_staggered.py:L2027-L2092

Impact: The previous centered reg rank-0 bug is fixed. _centered_or_bread() maps _safe_inv()’s all-NaN rank-0 sentinel to a zero matrix, so constant-only covariates collapse to the no-covariate reg IF rather than NaN-poisoning SEs. The new tests cover both unweighted and survey-weighted paths.

Concrete fix: None.

P3 — Documented methodology deviations are acceptable

Location: docs/methodology/REGISTRY.md:L501-L534, TODO.md:L35-L37

Impact: The remaining deviations or deferred parity work are explicitly documented: propensity-score clipping vs R drop-trimming, no-covariate IPW’s unconditional presentation, DR no-covariate per-cell SE, and fixture regeneration for IPW aggregation yardsticks. Under the review rules, these are not blockers.

Concrete fix: None required for this PR.

The DRDID source cross-check is consistent with the PR’s direction: reg_did_panel includes the OLS estimation-effect term and IF-based SE, while std_ipw_did_panel includes the PS estimation-effect correction and IF-based SE. citeturn3fetch0turn4fetch0

Code Quality

No blocking findings. The vectorized reg path, general/survey reg path, and IPW path now use the same IF-derived SE convention, and the NaN guards avoid the var_psi > 0 else 0.0 masking issue.

Performance

No findings. Hoisting the centered OR bread for balanced never-treated reg cells is a reasonable optimization and preserves the same algebra.

Maintainability

P3 — Stale comments around centered reg rank-0 behavior

Location: diff_diff/staggered.py:L1610-L1613, diff_diff/staggered.py:L2728-L2730

Impact: Comments still say a rank-0 reg bread yields NaN, but _centered_or_bread() now deliberately maps centered rank-0 to zero for reg. The code is correct; the comments could confuse future edits.

Concrete fix: Update those comments to distinguish centered reg bread rank-0 from [1, X] IPW/DR bread rank-0.

Tech Debt

P3 — Residual items tracked in TODO

Location: TODO.md:L35-L37

Impact: DR no-covariate per-cell SE parity and IPW fixture regeneration are tracked, deferrable items.

Concrete fix: None required for this PR.

Security

No findings. I saw no secret material or security-sensitive changes in the modified files.

Documentation / Tests

No blocking findings. The PR adds targeted DRDID IF reconstruction tests and the missing constant-only reg covariate tests from the prior review.

I could not run the tests because the environment has neither numpy nor pytest installed.

CI review P3: two comments still said "a rank-0 bread yields NaN" on the
reg paths, contradicting _centered_or_bread's zero-correction collapse.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4AzrFUMqJxcUumSH31mSr
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: e226c0f35b012079c0da3d45f964dc4a1afbd2ce


Overall Assessment

Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • Affected methods: CallawaySantAnna panel reg and ipw per-cell IF/SE paths, including no-covariate, covariate, survey, cluster, and aggregation consumers.
  • Methodology cross-check passed: the added reg OLS estimation-effect term and IPW propensity-score correction match the DRDID panel source direction.
  • Prior re-review concern remains resolved: rank-0 centered reg bread is intentionally mapped to a zero centered correction and documented/test-covered.
  • No new inline inference anti-pattern found; changed paths feed SEs through safe_inference / safe_inference_batch.
  • I could not run tests locally because this environment lacks both numpy and pytest; static AST parsing of changed Python files passed.

Methodology

No P0/P1 findings.

External source cross-check: DRDID::reg_did_panel builds the OLS linear representation, applies M1, assembles the IF, and computes analytical SE from that IF; DRDID::std_ipw_did_panel builds the PS score/Hessian representation, applies M2, assembles the IF, and computes SE from it. The implementation at diff_diff/staggered.py:L1580-L1622, diff_diff/staggered.py:L2688-L2741, and diff_diff/staggered.py:L2970-L3017 follows that contract. (rdrr.io)

P3 — Documented deviations are acceptable
Severity: P3 informational.
Impact: Remaining deviations are documented: propensity-score clipping vs R drop-trimming, no-covariate IPW’s unconditional presentation, and DR no-covariate per-cell SE parity deferred in TODO.md. See docs/methodology/REGISTRY.md:L501-L534 and TODO.md:L35-L37.
Concrete fix: None required for this PR.

Code Quality

No findings. The modified paths consistently compute per-cell SE from the same IF stored for aggregation, and NaN/zero SE inference is gated through shared helpers.

Performance

No findings. Hoisting the centered reg bread for balanced never-treated control designs at diff_diff/staggered.py:L1426-L1440 is consistent with the documented centered-basis algebra and avoids repeated matrix inversions.

Maintainability

No findings. The prior stale-comment issue is addressed: _centered_or_bread() and the reg IF comments now explain why rank-0 centered reg bread is benign, while [1, X] IPW/DR all-NaN breads still propagate NaN (diff_diff/staggered.py:L2615-L2635, tests/test_staggered.py:L1658-L1690).

Tech Debt

P3 — Deferred work is tracked
Severity: P3 informational.
Impact: Fixture regeneration for IPW aggregation yardsticks and residual CS-scaling follow-ups are tracked in TODO.md:L35-L37; the generator already emits the future JSON blocks at benchmarks/R/generate_csdid_test_values.R:L111-L140.
Concrete fix: None required for approval.

Security

No findings. A secret-pattern scan of the changed files did not identify credentials; matches were unrelated existing documentation text.

Documentation / Tests

P3 — Local verification unavailable
Severity: P3 informational.
Impact: I could not execute the targeted tests because pytest is not installed, and importing the package fails because numpy is missing. Static syntax parsing passed for diff_diff/staggered.py, tests/test_methodology_callaway.py, tests/test_staggered.py, and tests/test_csdid_ported.py.
Concrete fix: Ensure CI runs the added parity tests, especially TestDRDIDPanelIFParity, TestDRDIDGoldenIFReconstruction, and TestRegIpwIFBehavior.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 5, 2026
@igerber igerber merged commit 5a76d9c into main Jul 5, 2026
35 of 36 checks passed
@igerber igerber deleted the fix/cs-reg-ipw-if-se branch July 5, 2026 16:34
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