Skip to content

Commit fbdfe55

Browse files
igerberclaude
andcommitted
Address CI codex round 1 findings on PR igerber#477
P2 (Methodology — missing PSD/finite warning on combined survey meat): mirror `_compute_conley_meat`'s finite + negative-eigenvalue guard on the combined `meat = meat_spatial + meat_serial` returned by the survey panel-block orchestrator. Both the radial 1-D Bartlett spatial kernel AND the Newey-West Bartlett serial kernel are practitioner specializations that are NOT formally PSD-guaranteed; adding two non-PSD-guaranteed terms can produce a more indefinite combined meat, so the diagnostic surface matters more on the panel-block path than the no-survey baseline. Guard fires after the saturation NaN-fail check (so genuinely-saturated meats NaN-propagate without spurious warning). P3 (Documentation — public docs missing effective-PSU restriction): README.md, diff_diff/guides/llms.txt, and docs/api/spillover.rst now mention the effective-PSU requirement for `conley_lag_cutoff > 0` (weights-only / strata-only without cluster fallback raises NotImplementedError). REGISTRY's Restrictions list already had the caveat; the public-surface docs are now consistent. P3 (Wording — single-stratum reduction overclaim): REGISTRY and CHANGELOG single-stratum reduction descriptions clarified to "Conley sandwich on within-stratum-CENTERED PSU totals" (NOT plain). The Binder TSL centering is retained at H=1 — under survey weights the per-period stratum mean is always subtracted from PSU scores before the kernel application, even when the stratum is the entire sample. The cross-sectional Wave E.2 reduction wording was already correct (centered). P3 (Documentation — test_a `full meat matrix` overclaim): tightened test_a docstring to accurately describe what is pinned (ATT AND scalar SE bit-identity via assert_array_equal), with an explicit note that full meat-matrix equality is implied — not directly asserted — because the meat matrix is not exposed on `SpilloverDiDResults`. P3 (Tech debt — code duplication, DEFERRED): the serial Bartlett kernel logic is duplicated between two_stage.py (survey path) and conley.py (no-survey path). Factoring out a shared kernel helper + shared PSD/finite guard is cosmetic and out of scope for this PR. Added a TODO.md row to track the refactor follow-up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 34a01c1 commit fbdfe55

8 files changed

Lines changed: 60 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ Full guide: `diff_diff.get_llm_guide("practitioner")`.
106106
- [SunAbraham](https://diff-diff.readthedocs.io/en/stable/api/staggered.html) - Sun & Abraham (2021) interaction-weighted estimator for heterogeneity-robust event studies
107107
- [ImputationDiD](https://diff-diff.readthedocs.io/en/stable/api/imputation.html) - Borusyak, Jaravel & Spiess (2024) imputation estimator, most efficient under homogeneous effects
108108
- [TwoStageDiD](https://diff-diff.readthedocs.io/en/stable/api/two_stage.html) - Gardner (2022) two-stage estimator with GMM sandwich variance
109-
- [SpilloverDiD](https://diff-diff.readthedocs.io/en/stable/api/spillover.html) - Butts (2021) ring-indicator spillover-aware DiD identifying direct effect on treated + per-ring spillover on near-control units; handles non-staggered and staggered timing; supports survey-design variance under `survey_design=` for HC1 / CR1 (Wave E.1 Binder TSL) and Conley (Wave E.2 panel-aware stratified-Conley sandwich on per-period PSU totals; extended in Wave E.2 follow-up to `conley_lag_cutoff > 0` via panel-block composition with within-PSU serial Bartlett HAC)
109+
- [SpilloverDiD](https://diff-diff.readthedocs.io/en/stable/api/spillover.html) - Butts (2021) ring-indicator spillover-aware DiD identifying direct effect on treated + per-ring spillover on near-control units; handles non-staggered and staggered timing; supports survey-design variance under `survey_design=` for HC1 / CR1 (Wave E.1 Binder TSL) and Conley (Wave E.2 panel-aware stratified-Conley sandwich on per-period PSU totals; extended in Wave E.2 follow-up to `conley_lag_cutoff > 0` via panel-block composition with within-PSU serial Bartlett HAC`lag>0` requires an effective PSU via explicit `survey_design.psu` or injected `cluster=<col>`)
110110
- [SyntheticDiD](https://diff-diff.readthedocs.io/en/stable/api/estimators.html) - Synthetic DiD combining standard DiD and synthetic control for few treated units
111111
- [TripleDifference](https://diff-diff.readthedocs.io/en/stable/api/triple_diff.html) - triple difference (DDD) estimator for designs requiring two criteria for treatment eligibility
112112
- [ContinuousDiD](https://diff-diff.readthedocs.io/en/stable/api/continuous_did.html) - Callaway, Goodman-Bacon & Sant'Anna (2024) continuous treatment DiD with dose-response curves

TODO.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ Deferred items from PR reviews that were not addressed before merge.
139139
| `SyntheticDiD(vcov_type="conley")` support. Currently raises `TypeError` at `__init__` because SyntheticDiD uses `variance_method ∈ {bootstrap, jackknife, placebo}` rather than the analytical sandwich that Conley plugs into. Wiring would require either reimplementing an analytical sandwich path for SyntheticDiD or designing a spatial-block bootstrap (new methodology, Politis-Romano 1994 territory). | `synthetic_did.py::SyntheticDiD` | follow-up (spillover-conley) | Low |
140140
| `SpilloverDiD(survey_design=...)` replicate-weight variance (BRR / Fay / JK1 / JKn / SDR). Wave E.1 ships Taylor-linearization only. Per Gerber (2026) Appendix A, the IF-reweighting shortcut does NOT apply to TwoStageDiD-class estimators because `gamma_hat` is weight-sensitive; correct support requires per-replicate full re-fit of stage 1 and stage 2 (200+ LoC of test surface beyond E.1). | `spillover.py::SpilloverDiD.fit`, `survey.py::compute_replicate_refit_variance` | follow-up | Low |
141141
| `SpilloverDiD(survey_design=...)` subpopulation preservation (Wave E.3). Wave E.1's `finite_mask` block physically removes zero-weight rows that lose stage-1 FE support, so `SurveyDesign.subpopulation()`-derived designs see `n_psu` / `df_survey` / Binder centering recomputed on the reduced fit sample rather than the full domain design. Standard domain-estimation practice (R `survey::svyrecvar` on a `subset()` design) preserves the original PSU/strata counts and treats out-of-domain rows as zero-score padding. Fix requires separating fit-sample alignment (Psi array) from design-level bookkeeping: preserve a full-design `resolved_survey` for inference metadata + zero-pad dropped zero-weight rows' IF contribution. Add `SurveyDesign.subpopulation()` regression test to lock the contract. | `spillover.py::SpilloverDiD.fit`, `two_stage.py::_compute_binder_tsl_meat` | follow-up (Wave E.3) | Medium |
142+
| Serial Bartlett kernel logic duplicated between `diff_diff/two_stage.py::_compute_stratified_serial_bartlett_meat` (survey path) and `diff_diff/conley.py::_compute_conley_meat` panel-block branch (no-survey path). Both compute `K[t,s] = (1 - |t-s|/(L+1)) * 1{|t-s| <= L, t != s}` over dense panel-period codes. Factor out a shared `_serial_bartlett_kernel_matrix(t_codes, L)` helper and a shared post-meat finite + PSD-warning guard so the survey and no-survey paths can't drift on diagnostics or kernel weights. Cosmetic; refactor doesn't change behavior. | `two_stage.py::_compute_stratified_serial_bartlett_meat`, `conley.py::_compute_conley_meat` | follow-up | Low |
142143
| `SpilloverDiD(vcov_type="conley", conley_lag_cutoff > 0, survey_design=...)` no-effective-PSU serial Bartlett HAC. Wave E.2 follow-up ships the panel-block composition when an effective PSU exists (explicit `survey_design.psu` OR injected via `cluster=<col>` per `_inject_cluster_as_psu`). Weights-only / strata-only survey designs WITHOUT a cluster fallback raise `NotImplementedError` at `SpilloverDiD.fit` post-resolution because under the pseudo-PSU = obs-index fallback each pseudo-PSU appears in exactly one period — the per-PSU serial cross-period loop would silently contribute zero. Fix would either derive a unit-level serial fallback for no-PSU designs (mixes IF allocators with the pseudo-PSU spatial term — needs methodology work) or route the serial loop through `conley_unit` with explicit documentation of the IF-allocator asymmetry. Regression goldens vs the effective-PSU shipped path. | `spillover.py::SpilloverDiD.fit`, `two_stage.py::_compute_stratified_serial_bartlett_meat` | follow-up (Wave E.2 follow-up tail) | Low |
143144
| `SpilloverDiD(ring_method="count")` extension. Currently only the nearest-treated-ring specification is exposed. Count-of-treated-in-ring (paper Section 3.2 end) is methodologically supported by Butts but re-introduces functional-form dependence; expose with an explicit kwarg gate and documentation warning. | `spillover.py::SpilloverDiD.fit` | follow-up | Low |
144145
| `SpilloverDiD` data-driven `d_bar` selection (Butts 2021b / Butts 2023 JUE Insight cross-validation). | `spillover.py::SpilloverDiD` | follow-up | Low |

diff_diff/guides/llms.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ Full practitioner guide: call `diff_diff.get_llm_guide("practitioner")`
5858
- [SunAbraham](https://diff-diff.readthedocs.io/en/stable/api/staggered.html): Sun & Abraham (2021) interaction-weighted estimator for heterogeneity-robust event studies
5959
- [ImputationDiD](https://diff-diff.readthedocs.io/en/stable/api/imputation.html): Borusyak, Jaravel & Spiess (2024) imputation estimator — most efficient under homogeneous effects
6060
- [TwoStageDiD](https://diff-diff.readthedocs.io/en/stable/api/two_stage.html): Gardner (2022) two-stage estimator with GMM sandwich variance
61-
- [SpilloverDiD](https://diff-diff.readthedocs.io/en/stable/api/spillover.html): Butts (2021) ring-indicator spillover-aware DiD identifying direct effect on treated + per-ring spillover-on-control; reuses `conley_coords` for ring construction; handles non-staggered and staggered timing; supports `SurveyDesign(weights, strata, psu, fpc)` under `vcov_type="hc1"` with optional `cluster=<col>` for CR1 via Gerber (2026) Binder TSL (Wave E.1) and under `vcov_type="conley"` via a panel-aware stratified-Conley sandwich on per-period PSU totals (Wave E.2 cross-sectional `conley_lag_cutoff=0`) extended in Wave E.2 follow-up to `conley_lag_cutoff > 0` via panel-block composition with within-PSU serial Bartlett HAC (Newey-West 1987 separable form), both composed with the Wave D Gardner GMM correction (replicate weights queued as follow-up)
61+
- [SpilloverDiD](https://diff-diff.readthedocs.io/en/stable/api/spillover.html): Butts (2021) ring-indicator spillover-aware DiD identifying direct effect on treated + per-ring spillover-on-control; reuses `conley_coords` for ring construction; handles non-staggered and staggered timing; supports `SurveyDesign(weights, strata, psu, fpc)` under `vcov_type="hc1"` with optional `cluster=<col>` for CR1 via Gerber (2026) Binder TSL (Wave E.1) and under `vcov_type="conley"` via a panel-aware stratified-Conley sandwich on per-period PSU totals (Wave E.2 cross-sectional `conley_lag_cutoff=0`) extended in Wave E.2 follow-up to `conley_lag_cutoff > 0` via panel-block composition with within-PSU serial Bartlett HAC (Newey-West 1987 separable form; `lag>0` requires an effective PSU via explicit `survey_design.psu` or injected `cluster=<col>`), both composed with the Wave D Gardner GMM correction (replicate weights queued as follow-up)
6262
- [SyntheticDiD](https://diff-diff.readthedocs.io/en/stable/api/estimators.html): Synthetic DiD combining standard DiD and synthetic control methods for few treated units
6363
- [TripleDifference](https://diff-diff.readthedocs.io/en/stable/api/triple_diff.html): Triple difference (DDD) estimator for designs requiring two criteria for treatment eligibility
6464
- [ContinuousDiD](https://diff-diff.readthedocs.io/en/stable/api/continuous_did.html): Callaway, Goodman-Bacon & Sant'Anna (2024) continuous treatment DiD with dose-response curves

diff_diff/two_stage.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,36 @@ def _compute_stratified_conley_meat(
838838
)
839839
return np.full((p_2, p_2), np.nan)
840840

841+
# Finite + PSD guards on the COMBINED survey meat (spatial + serial).
842+
# Mirrors :func:`diff_diff.conley._compute_conley_meat` L966-990 so the
843+
# survey panel-block path has the same diagnostic surface as the
844+
# no-survey path. The radial 1-D Bartlett spatial kernel and the
845+
# Newey-West Bartlett serial kernel are both practitioner
846+
# specializations that are NOT formally PSD-guaranteed; adding two
847+
# non-PSD-guaranteed terms can produce a more indefinite combined
848+
# meat, so the check matters most on the panel-block path. CI codex
849+
# R1 P2 fix.
850+
if not np.all(np.isfinite(meat)):
851+
raise ValueError(
852+
"SpilloverDiD Wave E.2 stratified-Conley meat contains non-finite "
853+
"values; check Psi for NaN/Inf upstream of the sandwich."
854+
)
855+
eigvals = np.linalg.eigvalsh(meat)
856+
if eigvals.size and eigvals.min() < -1e-12:
857+
warnings.warn(
858+
f"SpilloverDiD Wave E.2 stratified-Conley meat with conley_kernel="
859+
f"{conley_kernel!r} has a materially negative eigenvalue "
860+
f"({eigvals.min():.2e}); the variance estimator is not guaranteed "
861+
"PSD on this design. Both supported kernels (radial bartlett and "
862+
"uniform spatial) plus the hardcoded serial Bartlett term are "
863+
"practitioner specializations of Conley 1999 / Newey-West 1987 "
864+
"and are not formally PSD-guaranteed; consider varying "
865+
"conley_cutoff_km / conley_lag_cutoff, or reviewing the design "
866+
"for collinearity / degenerate residual structure.",
867+
UserWarning,
868+
stacklevel=2,
869+
)
870+
841871
return meat
842872

843873

docs/api/spillover.rst

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,14 @@ and planned follow-up enhancements:
330330
``conley_lag_cutoff > 0`` panel-block composition).** SHIPPED in
331331
Wave E.2 follow-up. ``vcov_type="conley" + conley_lag_cutoff > 0 +
332332
survey_design=`` is now supported by adding a within-PSU serial
333-
Bartlett HAC term to the Wave E.2 spatial sandwich.
333+
Bartlett HAC term to the Wave E.2 spatial sandwich, **provided the
334+
survey design has an effective PSU** (either explicit
335+
``survey_design.psu`` or a ``cluster=<col>`` argument that gets
336+
injected as the effective PSU per Wave E.1's
337+
``_inject_cluster_as_psu`` routing). No-effective-PSU survey designs
338+
(weights-only / strata-only WITHOUT a cluster fallback) raise
339+
``NotImplementedError`` at ``SpilloverDiD.fit`` post-resolution —
340+
see the Restrictions list below.
334341

335342
.. note::
336343

@@ -390,6 +397,14 @@ and planned follow-up enhancements:
390397

391398
Restrictions:
392399

400+
- **Requires an effective PSU** — either explicit ``survey_design.psu``
401+
OR ``cluster=<col>`` injected as the effective PSU per Wave E.1's
402+
``_inject_cluster_as_psu``. No-effective-PSU survey designs
403+
(weights-only / strata-only WITHOUT a cluster fallback) raise
404+
``NotImplementedError`` post-resolution at ``SpilloverDiD.fit``
405+
because the pseudo-PSU = obs-index fallback would silently zero
406+
the serial sum (each pseudo-PSU appears in exactly one period).
407+
Tracked as a follow-up in ``TODO.md``.
393408
- Replicate-weight variance (BRR / Fay / JK1 / JKn / SDR) raises
394409
``NotImplementedError`` (inherits Wave E.1 gate).
395410
- DiagnosticReport routing for the panel-block case is queued for the

docs/methodology/REGISTRY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3289,7 +3289,7 @@ Degrees of freedom for the t-distribution lookup use `ResolvedSurveyDesign.df_su
32893289
- **Reduction semantics (load-bearing for tests):**
32903290
- `conley_lag_cutoff = 0` or `None`: **bit-identical** to shipped Wave E.2 (orchestrator early-returns BEFORE invoking the serial helper; `assert_array_equal` regression pin at test_a).
32913291
- `conley_time is None` or `T = 1`: serial helper short-circuits to zero meat (no cross-period pairs possible) — the degenerate panel-block path, NOT a saturation diagnostic.
3292-
- Single stratum (`H = 1`, `FPC = inf`): spatial reduces to `sum_t` plain Conley sandwich on per-period PSU totals; serial reduces to plain Newey-West Bartlett HAC on PSU totals (modulo the panel-wide `G/(G-1)` survey factor).
3292+
- Single stratum (`H = 1`, `FPC = inf`): spatial reduces to `sum_t` Conley sandwich on per-period **within-stratum-CENTERED** PSU totals (NOT raw — at H=1 the centering still subtracts the per-period mean over all G PSUs); serial reduces to Newey-West Bartlett HAC on per-period within-stratum-CENTERED PSU totals (NOT raw scores; the survey-weighted form retains Binder TSL centering even at H=1). Both reductions carry the panel-wide `G/(G-1)` survey factor in lieu of FPC.
32933293
- Bandwidth → 0, `L > 0`: spatial reduces to `sum_t` per-period within-stratum HC sandwich on PSU totals; serial term unchanged (separable form).
32943294
- All PSUs singleton in stratum + `lonely_psu="remove"`: `df_survey = 0` and the meat NaN-fails (saturation warning template "Wave E.2 stratified-Conley sandwich" covers both cross-sectional and panel-block cases).
32953295
- Cross-stratum kernel weight is exactly zero on the serial term too (the within-PSU loop is necessarily within-stratum).

tests/test_spillover.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6481,16 +6481,20 @@ def _fit(self, df, lag_cutoff=1, design=None, **kwargs):
64816481
)
64826482

64836483
def test_a_lag0_strict_bit_identical_to_wave_e2_meat(self):
6484-
"""`conley_lag_cutoff = 0` MUST be bit-identical to shipped Wave E.2
6485-
on the full meat matrix (not just SE scalar — a bug where SE coincides
6486-
but meat[i, j] differs would pass a scalar pin). Orchestrator must
6487-
short-circuit BEFORE calling the serial helper.
6484+
"""`conley_lag_cutoff = 0` MUST produce bit-identical ATT AND scalar SE
6485+
as a fresh Wave E.2 baseline fit (`assert_array_equal`). Orchestrator
6486+
must short-circuit BEFORE calling the serial helper; test_a2 mock-spy
6487+
verifies the helper isn't invoked.
64886488
64896489
Methodology lock: the early-return at the orchestrator level is the
64906490
backwards-compatibility guarantee that the shipped Wave E.2 surface
64916491
is unaffected by the follow-up. Without the early-return, a numerical
64926492
zero from the serial helper would still inject floating-point noise
6493-
into the spatial-only meat.
6493+
into the spatial-only meat (which would surface as SE drift).
6494+
6495+
Note: full meat-matrix equality is implied (ATT + SE bit-identity is
6496+
load-bearing for the user-visible regression pin; the meat matrix is
6497+
not directly exposed on `SpilloverDiDResults`).
64946498
"""
64956499
df = generate_butts_nonstaggered_dgp(seed=0)
64966500
df_s = _augment_with_survey(df, n_strata=2, psus_per_stratum=4, fpc=200.0)

0 commit comments

Comments
 (0)