Skip to content

Commit efb5252

Browse files
igerberclaude
andcommitted
Address PR #364 AI review R5: placebo_event_study NaN-on-invalid parity
P1: the NaN-on-invalid-bootstrap pattern was applied to overall / joiners / leavers / event_study_effects / path_effects, but the parallel dynamic-placebo propagation block at chaisemartin_dhaultfoeuille.py:3050 kept the pre-fix pattern (`if np.isfinite(bs_se): overwrite else leave analytical intact`). Dynamic placebo rows surface on `results.placebo_event_study` and in `results.to_dataframe(level="event_study")` negative-horizon rows; a non-finite placebo bootstrap SE would silently leave analytical `se / t_stat / p_value / conf_int` in place, mixing bootstrap- and analytical-contract semantics in the same rendered output. Fix: add the `else -> NaN tuple` branch to the placebo_event_study propagation block, mirroring the five other surfaces above. Regression test `test_nan_contract_extends_to_placebo_event_study_horizons` fits `n_bootstrap=1` on a T=5 panel with placebos enabled and asserts that every `placebo_event_study[-lag]` entry and every negative-horizon row in `to_dataframe(level="event_study")` has NaN `se / t_stat / p_value / conf_int_{lower,upper}`. Full dCDH regression: 215 pass. TestByPathBootstrap: 15 pass under -m slow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 62578ac commit efb5252

2 files changed

Lines changed: 100 additions & 1 deletion

File tree

diff_diff/chaisemartin_dhaultfoeuille.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3034,8 +3034,20 @@ def fit(
30343034
if bootstrap_results.placebo_horizon_p_values
30353035
else None
30363036
)
3037+
# Same bootstrap-contract rule as overall / joiners /
3038+
# leavers / event_study_effects / path_effects above:
3039+
# once the caller opts into n_bootstrap > 0, the
3040+
# bootstrap output replaces analytical inference on
3041+
# this surface regardless of outcome. Non-finite
3042+
# bootstrap SE writes NaN to the full inference tuple
3043+
# rather than silently leaving analytical values in
3044+
# place — that would mix bootstrap-contract and
3045+
# analytical-contract semantics in the same rendered
3046+
# output (dynamic placebo rows appear in
3047+
# `results.to_dataframe(level="event_study")` alongside
3048+
# positive-horizon entries).
3049+
eff = placebo_event_study_dict[neg_key]["effect"]
30373050
if bs_se is not None and np.isfinite(bs_se):
3038-
eff = placebo_event_study_dict[neg_key]["effect"]
30393051
placebo_event_study_dict[neg_key]["se"] = bs_se
30403052
placebo_event_study_dict[neg_key]["p_value"] = (
30413053
bs_p if bs_p is not None else np.nan
@@ -3049,6 +3061,13 @@ def fit(
30493061
alpha=self.alpha,
30503062
df=_inference_df(_df_survey, resolved_survey),
30513063
)[0]
3064+
else:
3065+
placebo_event_study_dict[neg_key]["se"] = np.nan
3066+
placebo_event_study_dict[neg_key]["p_value"] = np.nan
3067+
placebo_event_study_dict[neg_key]["conf_int"] = (
3068+
np.nan, np.nan,
3069+
)
3070+
placebo_event_study_dict[neg_key]["t_stat"] = np.nan
30523071

30533072
# Phase 2: build normalized_effects with SE
30543073
normalized_effects_out: Optional[Dict[int, Dict[str, Any]]] = None

tests/test_chaisemartin_dhaultfoeuille.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4593,6 +4593,86 @@ def test_overflow_warning_fires_exactly_once_under_bootstrap(self):
45934593
f"Messages: {[str(w.message) for w in overflow_warnings]}"
45944594
)
45954595

4596+
def test_nan_contract_extends_to_placebo_event_study_horizons(self):
4597+
"""
4598+
Dynamic placebo horizons go through their own bootstrap
4599+
propagation block at
4600+
``chaisemartin_dhaultfoeuille.py::placebo_event_study_dict``
4601+
and surface in ``results.placebo_event_study`` and
4602+
``results.to_dataframe(level="event_study")`` (negative-horizon
4603+
rows). Pin the same NaN-on-invalid contract as the positive
4604+
horizons: ``n_bootstrap=1`` on a panel with valid placebo
4605+
eligibility must yield NaN SE / t / p / CI on every placebo
4606+
entry, not the analytical values populated in the build step
4607+
before bootstrap propagation.
4608+
"""
4609+
# Longer panel (T=5) so placebo horizons have enough cells.
4610+
rng = np.random.default_rng(42)
4611+
rows = []
4612+
for g in (1, 2, 3, 4, 5, 6):
4613+
for t in range(5):
4614+
d = 1 if t >= 2 else 0
4615+
y = d * 2.0 + rng.normal(0, 0.1)
4616+
rows.append({"group": g, "period": t, "treatment": d, "outcome": y})
4617+
for g in (7, 8):
4618+
for t in range(5):
4619+
y = rng.normal(0, 0.1)
4620+
rows.append({"group": g, "period": t, "treatment": 0, "outcome": y})
4621+
data = pd.DataFrame(rows)
4622+
4623+
with warnings.catch_warnings():
4624+
warnings.simplefilter("ignore")
4625+
est = ChaisemartinDHaultfoeuille(
4626+
n_bootstrap=1, # forces non-finite bootstrap SE
4627+
seed=42,
4628+
twfe_diagnostic=False,
4629+
placebo=True, # enable placebo surface
4630+
)
4631+
res = est.fit(
4632+
data,
4633+
outcome="outcome",
4634+
group="group",
4635+
time="period",
4636+
treatment="treatment",
4637+
L_max=2,
4638+
)
4639+
4640+
# If the panel + L_max produced placebo horizons, each must be
4641+
# NaN-consistent. If no placebos were produced, skip — the test
4642+
# relies on having at least one placebo row to exercise the
4643+
# propagation path.
4644+
if res.placebo_event_study is None or not res.placebo_event_study:
4645+
pytest.skip(
4646+
"placebo_event_study empty on this panel; cannot exercise "
4647+
"the placebo bootstrap propagation path"
4648+
)
4649+
for lag_key, entry in res.placebo_event_study.items():
4650+
assert np.isnan(entry["se"]), (
4651+
f"placebo_event_study[{lag_key}].se must be NaN under "
4652+
f"n_bootstrap=1; got {entry['se']}"
4653+
)
4654+
assert np.isnan(entry["t_stat"])
4655+
assert np.isnan(entry["p_value"])
4656+
lo, hi = entry["conf_int"]
4657+
assert np.isnan(lo) and np.isnan(hi)
4658+
# Effect may be NaN legitimately when N_pl_l == 0 for this
4659+
# lag (panel/horizon eligibility, not a bootstrap artifact).
4660+
# We only assert the inference-field NaN contract here.
4661+
4662+
# `to_dataframe(level="event_study")` surfaces these rows too.
4663+
# Negative-horizon rows must also show NaN in the inference
4664+
# columns.
4665+
df_es = res.to_dataframe(level="event_study")
4666+
negative_rows = df_es[df_es["horizon"] < 0]
4667+
if len(negative_rows) > 0:
4668+
for col in ("se", "t_stat", "p_value",
4669+
"conf_int_lower", "conf_int_upper"):
4670+
assert negative_rows[col].isna().all(), (
4671+
f"to_dataframe(level='event_study') negative-horizon "
4672+
f"column {col!r} must be NaN under n_bootstrap=1; "
4673+
f"got {negative_rows[col].tolist()}"
4674+
)
4675+
45964676
def test_summary_footer_mixed_validity_surfaces_live_targets(self):
45974677
"""
45984678
Mixed-validity case: overall_se / event_study_ses degenerate to

0 commit comments

Comments
 (0)