Skip to content

Commit 096a720

Browse files
igerberclaude
andcommitted
fix(inference): fail closed to NaN on non-finite degrees of freedom
The CR2/Bell-McCaffrey guard added in the previous commit can surface a non-finite (NaN) Satterthwaite DOF for a high-leverage / collinear nuisance coefficient. Two inference paths did not treat that as undefined: - `safe_inference()` rejected `df <= 0` but let a NaN `df` through (`NaN <= 0` is False), computing a finite t-stat with a NaN p-value / CI - an inconsistent, partially-finite inference tuple. It now returns all-NaN for any non-finite `df`. - `MultiPeriodDiD(vcov_type="hc2_bm")` fell back to the shared residual `df` for a period effect whose per-coefficient BM DOF was non-finite, reporting finite t/p/CI for a coefficient whose DOF was declared unreliable. It now passes the BM DOF through unconditionally on the hc2_bm path, so a non-finite DOF fails closed via `safe_inference`. The post-period-average and SunAbraham contrast paths already route their DOF through `safe_inference`, so the central guard covers them too. Only coefficients whose BM DOF is non-finite are affected; well- conditioned treatment / event-study / average contrasts are unchanged. Also rewrites the REGISTRY `DID_M^pl` placebo formula to display the backward-difference x switch-direction convention (`stable0_avg - joiner_avg` / `leaver_avg - stable1_avg`) so it matches the code and the adjacent sign note. Regression tests: non-finite `df` in `safe_inference`; monkeypatched NaN BM DOF failing closed on user-facing MPD period and average contrasts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0178r8ZK6VRiXbaBiknrWjQB
1 parent e26a51c commit 096a720

6 files changed

Lines changed: 91 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4040
`survey_design=SurveyDesign(psu=<cluster_col>)`. No behavior change for unclustered fits.
4141

4242
### Fixed
43+
- **Non-finite degrees of freedom now fail closed to all-NaN inference.**
44+
`safe_inference()` previously rejected `df <= 0` but let a non-finite `df` (NaN) through,
45+
producing an inconsistent tuple (finite t-stat, NaN p-value/CI). It now returns all-NaN for
46+
any non-finite `df`. `MultiPeriodDiD(vcov_type="hc2_bm")` likewise no longer falls back to
47+
the shared residual `df` when a coefficient's Bell-McCaffrey Satterthwaite DOF is non-finite
48+
(guard-suppressed) — such a coefficient's inference is now NaN rather than silently computed
49+
from a different `df`, preserving the joint-NaN inference contract. Only affects coefficients
50+
whose BM DOF was declared unreliable (high-leverage / collinear); well-conditioned
51+
treatment / event-study / average contrasts are unchanged.
4352
- **Unweighted clustered CR2 / Bell-McCaffrey per-coefficient Satterthwaite DOF no
4453
longer returns non-physical values** for high-leverage FE-dummy / collinear nuisance
4554
columns. The simple `(tr B)²/tr(B²)` form could produce a garbage DOF there (float64

diff_diff/estimators.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2132,11 +2132,16 @@ def _refit_mp_absorb(w_r):
21322132
idx = interaction_indices[period]
21332133
effect = coefficients[idx]
21342134
se = np.sqrt(vcov[idx, idx])
2135-
# Prefer per-coefficient BM DOF when available (hc2_bm path);
2136-
# otherwise fall back to the shared analytical df.
2137-
period_df = df
2138-
if _bm_dof_per_coef is not None and np.isfinite(_bm_dof_per_coef[idx]):
2135+
# Use the per-coefficient BM DOF when available (hc2_bm path);
2136+
# otherwise fall back to the shared analytical df. On the hc2_bm path
2137+
# the coefficient's OWN Bell-McCaffrey DOF governs its inference: a
2138+
# non-finite (guard-suppressed, unreliable) BM DOF makes t/p/CI
2139+
# undefined, so pass it through to `safe_inference` (which returns
2140+
# all-NaN) rather than silently falling back to the residual df.
2141+
if _bm_dof_per_coef is not None:
21392142
period_df = float(_bm_dof_per_coef[idx])
2143+
else:
2144+
period_df = df
21402145
t_stat, p_value, conf_int = safe_inference(effect, se, alpha=self.alpha, df=period_df)
21412146

21422147
period_effects[period] = PeriodEffect(

diff_diff/utils.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,13 @@ def safe_inference(effect, se, alpha=0.05, df=None):
352352
"""
353353
if not (np.isfinite(se) and se > 0):
354354
return np.nan, np.nan, (np.nan, np.nan)
355-
if df is not None and df <= 0:
356-
# Undefined degrees of freedom (e.g., rank-deficient replicate design)
355+
if df is not None and not (np.isfinite(df) and df > 0):
356+
# Undefined degrees of freedom: df <= 0 (e.g., rank-deficient replicate
357+
# design) OR non-finite (a guard-suppressed / non-physical Bell-McCaffrey
358+
# Satterthwaite DOF, which `_cr2_bm_dof_inner` returns as NaN for
359+
# high-leverage nuisance contrasts). All inference fields are NaN so a
360+
# coefficient whose DOF was declared unreliable never reports finite
361+
# t/p/CI - preserving the joint-NaN inference contract.
357362
return np.nan, np.nan, (np.nan, np.nan)
358363
t_stat = effect / se
359364
p_value = compute_p_value(t_stat, df=df)

docs/methodology/REGISTRY.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -744,15 +744,17 @@ DID_+ = sum_{t>=2} (N_{1,0,t} / sum_{t} N_{1,0,t}) * DID_{+,t}
744744
DID_- = sum_{t>=2} (N_{0,1,t} / sum_{t} N_{0,1,t}) * DID_{-,t}
745745
```
746746

747-
Single-lag placebo (AER 2020 placebo specification, same section as Theorem 3) — applies the same Theorem 3 logic to `Y_{g,t-1} - Y_{g,t-2}` on cells with 3-period histories:
747+
Single-lag placebo (AER 2020 placebo specification, same section as Theorem 3) — applies the same Theorem 3 logic to the pre-period first difference on cells with 3-period histories. Writing `d_a(cells) = mean over `cells` of (Y_{g,t-1} - Y_{g,t-2})` for the pre-period forward first difference:
748748

749749
```
750750
DID_M^pl = (1/N_S^pl) * sum_{t>=3} (
751-
N_{1,0,t} * [(Y_{g,t-1} - Y_{g,t-2})_{joiners} - ...] +
752-
N_{0,1,t} * [(Y_{g,t-1} - Y_{g,t-2})_{stable_1} - ...]
751+
N_{1,0,t} * [ d_a(stable_0(t)) - d_a(joiners(t)) ] # joiners side, S=+1
752+
+ N_{0,1,t} * [ d_a(leavers(t)) - d_a(stable_1(t)) ] # leavers side, S=-1
753753
)
754754
```
755755

756+
The per-side terms are the code's `placebo_plus_t = stable0_avg - joiner_avg` and `placebo_minus_t = leaver_avg - stable1_avg` — the backward-difference × switch-direction convention of the sign Note below (equivalently, `S_g · (Y_{g,t-2} - Y_{g,t-1})_switcher` minus the same for its stable controls, matching `_compute_multi_horizon_placebos` and R).
757+
756758
**Note (sign convention):** the reported `placebo_effect` uses the **backward-difference × switch-direction** convention of the multi-horizon placebo path (`_compute_multi_horizon_placebos`: `switcher_change - ctrl_avg` with `Y_bwd - Y_ref`, times `S_g = +1` joiners / `-1` leavers) and R `did_multiplegt_dyn`. In the phase-1 (`L_max=None`) code this is `placebo_plus_t = stable0_avg - joiner_avg` (joiners) and `placebo_minus_t = leaver_avg - stable1_avg` (leavers). Prior to this the phase-1 path used the opposite (forward-difference) order, so `placebo_effect` was **sign-flipped vs R** on pure-direction panels (magnitude bit-identical); the multi-horizon path was always correct. Pinned by `tests/test_chaisemartin_dhaultfoeuille_parity.py::TestDCDHDynRParity::test_parity_{joiners,leavers}_only`. On **mixed-direction** panels the placebo magnitude additionally carries the documented period-vs-cohort stable-control-set / equal-cell-weighting deviation (see the `**Note (deviation from R DIDmultiplegtDYN):**` above), so it is not gate-asserted against R there.
757759

758760
*Phase 2: Multi-horizon event study (Equation 3 and 5 of the dynamic companion paper):*

tests/test_estimators_vcov_type.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,52 @@ def test_multi_period_cluster_plus_hc2_bm_produces_finite_inference(self):
544544
assert np.isfinite(post_pe.se)
545545
assert np.isfinite(post_pe.p_value)
546546

547+
def test_multi_period_hc2_bm_nonfinite_bm_dof_fails_closed(self, monkeypatch):
548+
"""A non-finite (guard-suppressed) Bell-McCaffrey DOF must fail closed to
549+
NaN inference on user-facing MPD contrasts, not fall back to the shared
550+
residual df. The `_cr2_bm_dof_inner` guard can NaN a per-coefficient DOF
551+
(high-leverage / collinear column); reporting finite t/p/CI for such a
552+
coefficient via the residual df would violate the joint-NaN inference
553+
contract. Monkeypatches every BM DOF to NaN and asserts SEs stay finite
554+
(vcov is unaffected) while all t/p/CI fields are NaN.
555+
"""
556+
import numpy as np
557+
558+
import diff_diff.linalg as _linalg
559+
560+
rng = np.random.default_rng(2)
561+
rows = []
562+
for i in range(20):
563+
treated = int(i >= 10)
564+
for t in range(3):
565+
y = rng.normal(0.0, 1.0) + 0.5 * treated * (t >= 1)
566+
rows.append({"unit": i, "time": t, "treated": treated, "y": y})
567+
data = pd.DataFrame(rows)
568+
569+
_orig = _linalg._compute_cr2_bm_vcov_and_dof
570+
571+
def _nan_dof(*args, **kwargs):
572+
vcov, dof = _orig(*args, **kwargs)
573+
return vcov, np.full(np.asarray(dof).shape, np.nan)
574+
575+
# MPD's hc2_bm+cluster path imports this from linalg inside fit(), so the
576+
# local import binds to the patched attribute at call time.
577+
monkeypatch.setattr(_linalg, "_compute_cr2_bm_vcov_and_dof", _nan_dof)
578+
579+
res = MultiPeriodDiD(vcov_type="hc2_bm", cluster="unit").fit(
580+
data, outcome="y", treatment="treated", time="time"
581+
)
582+
for p, pe in res.period_effects.items():
583+
assert np.isfinite(pe.se), f"period {p}: SE should remain finite"
584+
assert np.isnan(pe.t_stat), f"period {p}: t_stat should be NaN"
585+
assert np.isnan(pe.p_value), f"period {p}: p_value should be NaN"
586+
assert np.isnan(pe.conf_int[0]) and np.isnan(pe.conf_int[1])
587+
# The post-period-average contrast fails closed too.
588+
assert np.isfinite(res.avg_se), "avg SE should remain finite"
589+
assert np.isnan(res.avg_t_stat)
590+
assert np.isnan(res.avg_p_value)
591+
assert np.isnan(res.avg_conf_int[0]) and np.isnan(res.avg_conf_int[1])
592+
547593
def test_multi_period_cluster_hc2_bm_avg_att_uses_clubsandwich_dof(self):
548594
"""MPD(cluster=..., hc2_bm) `avg_att` inference uses the new
549595
cluster-aware contrast Satterthwaite DOF, not the shared n-k fallback.

tests/test_utils.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,21 @@ def test_neg_inf_se_returns_all_nan(self):
586586
assert np.isnan(ci_lower)
587587
assert np.isnan(ci_upper)
588588

589+
def test_nonfinite_df_returns_all_nan(self):
590+
"""Non-finite df (NaN/inf) produces all-NaN inference even with a valid SE.
591+
592+
A guard-suppressed / non-physical Bell-McCaffrey Satterthwaite DOF is
593+
surfaced as NaN by `_cr2_bm_dof_inner`; a coefficient whose DOF was
594+
declared unreliable must not report finite (or partially-finite) t/p/CI.
595+
`df <= 0` is already rejected; this covers the non-finite case where
596+
`df <= 0` is False for NaN.
597+
"""
598+
for bad_df in (np.nan, np.inf, -np.inf):
599+
t_stat, p_value, (ci_lower, ci_upper) = safe_inference(5.0, 2.0, df=bad_df)
600+
assert np.isnan(t_stat), f"df={bad_df}: t_stat should be NaN"
601+
assert np.isnan(p_value), f"df={bad_df}: p_value should be NaN"
602+
assert np.isnan(ci_lower) and np.isnan(ci_upper), f"df={bad_df}: CI should be NaN"
603+
589604
def test_valid_se_normal_distribution(self):
590605
"""Test valid SE with normal distribution (df=None)."""
591606
effect = 5.0

0 commit comments

Comments
 (0)