Skip to content

Commit 62578ac

Browse files
igerberclaude
andcommitted
Address PR #364 AI review R4: footer covers joiners/leavers/path_effects
P2: the bootstrap footer in summary() previously keyed "produced non-finite SE on every target" only off overall_se and event_study_effects, missing joiners_se / leavers_se / path_effects. Because by_path zeros switcher-side contributions outside the selected path while keeping controls intact, a per-path bootstrap target can remain finite even when the overall/event-study target degenerates (e.g., on a reversible panel where the mix of joiners + leavers produces a zero centered IF). Without the broader predicate the footer would falsely claim blanket failure while a finite per-path SE sits in the rendered output below. Fix: add joiners_has_finite_bootstrap_se, leavers_has_finite_bootstrap_se, and path_effects_has_finite_bootstrap_se guards, and a new mixed-validity branch between the "used for event-study horizon inference" branch and the blanket-failure branch: "Note: bootstrap (N iterations) produced non-finite SE on the overall/event-study target; {joiners, leavers, per-path} bootstrap inference is populated." Regression test test_summary_footer_mixed_validity_surfaces_live_targets forces overall_se / event_study_effects to NaN post-fit on a healthy bootstrap fit (keeping path_effects finite), then asserts (a) the footer does not claim blanket failure, (b) does not claim "multiplier-bootstrap percentile inference" when overall is NaN, and (c) mentions "per-path" + "bootstrap inference is populated". Full dCDH regression: 215 pass. TestByPathBootstrap: 14 pass under -m slow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 21cffa4 commit 62578ac

2 files changed

Lines changed: 116 additions & 1 deletion

File tree

diff_diff/chaisemartin_dhaultfoeuille_results.py

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -710,14 +710,45 @@ def summary(self, alpha: Optional[float] = None) -> str:
710710
# least one horizon has a finite SE before claiming bootstrap was
711711
# "used for event-study horizon inference" — otherwise every
712712
# bootstrap inference field is NaN and we fall through to the
713-
# "bootstrap attempted but invalid" note.
713+
# "bootstrap attempted but invalid" note. The `any_finite_*`
714+
# predicate below expands the check to cover joiners / leavers /
715+
# path_effects too: by_path zeros switcher contributions for non-
716+
# path groups while keeping controls intact, so a per-path
717+
# bootstrap target can produce a finite SE even when the overall
718+
# / event-study bootstrap is degenerate (e.g., a reversible panel
719+
# where the overall mix of joiners + leavers produces a zero
720+
# centered IF while individual paths do not). Without this
721+
# broader predicate, the footer would falsely claim "produced
722+
# non-finite SE on every target" while a finite per-path
723+
# bootstrap SE sits in the rendered output below.
714724
event_study_has_finite_bootstrap_se = (
715725
self.event_study_effects is not None
716726
and any(
717727
np.isfinite(entry.get("se", np.nan))
718728
for entry in self.event_study_effects.values()
719729
)
720730
)
731+
joiners_has_finite_bootstrap_se = (
732+
self.joiners_se is not None and np.isfinite(self.joiners_se)
733+
)
734+
leavers_has_finite_bootstrap_se = (
735+
self.leavers_se is not None and np.isfinite(self.leavers_se)
736+
)
737+
path_effects_has_finite_bootstrap_se = (
738+
self.path_effects is not None
739+
and any(
740+
np.isfinite(h.get("se", np.nan))
741+
for entry in self.path_effects.values()
742+
for h in entry.get("horizons", {}).values()
743+
)
744+
)
745+
any_finite_bootstrap_inference = (
746+
np.isfinite(self.overall_se)
747+
or event_study_has_finite_bootstrap_se
748+
or joiners_has_finite_bootstrap_se
749+
or leavers_has_finite_bootstrap_se
750+
or path_effects_has_finite_bootstrap_se
751+
)
721752
if self.bootstrap_results is not None and np.isfinite(self.overall_se) and not is_delta:
722753
lines.append("Note: p-value and CI are multiplier-bootstrap percentile inference")
723754
lines.append(
@@ -738,6 +769,23 @@ def summary(self, alpha: Optional[float] = None) -> str:
738769
f"Note: bootstrap ({self.bootstrap_results.n_bootstrap} iterations) "
739770
f"used for event-study horizon inference."
740771
)
772+
elif self.bootstrap_results is not None and any_finite_bootstrap_inference:
773+
# Overall / event-study degenerated but joiners / leavers /
774+
# path_effects still have finite bootstrap SE. Point the reader
775+
# at the targets that succeeded rather than claiming a blanket
776+
# failure.
777+
live_targets = []
778+
if joiners_has_finite_bootstrap_se:
779+
live_targets.append("joiners")
780+
if leavers_has_finite_bootstrap_se:
781+
live_targets.append("leavers")
782+
if path_effects_has_finite_bootstrap_se:
783+
live_targets.append("per-path")
784+
lines.append(
785+
f"Note: bootstrap ({self.bootstrap_results.n_bootstrap} iterations) "
786+
f"produced non-finite SE on the overall/event-study target; "
787+
f"{', '.join(live_targets)} bootstrap inference is populated."
788+
)
741789
elif self.bootstrap_results is not None:
742790
lines.append(
743791
f"Note: bootstrap ({self.bootstrap_results.n_bootstrap} iterations) "

tests/test_chaisemartin_dhaultfoeuille.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4593,6 +4593,73 @@ 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_summary_footer_mixed_validity_surfaces_live_targets(self):
4597+
"""
4598+
Mixed-validity case: overall_se / event_study_ses degenerate to
4599+
NaN while joiners_se / leavers_se / path_effects horizons retain
4600+
finite bootstrap inference. ``by_path`` zeros switcher-side
4601+
contributions outside the selected path while keeping the control
4602+
pool intact, so path-level bootstrap targets can stay finite even
4603+
when the overall/event-study IF degenerates on a reversible
4604+
panel. The footer must point the reader at the live targets
4605+
rather than falsely claiming "non-finite SE on every target."
4606+
4607+
Uses a healthy bootstrap fit and post-hoc mutates overall_se /
4608+
event_study_effects to NaN, pinning the footer logic in
4609+
isolation from the (hard-to-engineer) natural reversible DGP
4610+
that produces this exact mixed-validity state.
4611+
"""
4612+
data = _by_path_three_path_data()
4613+
_est, res = self._fit_with_bootstrap(
4614+
data, by_path=3, L_max=3, n_bootstrap=200, seed=42,
4615+
)
4616+
# Sanity: healthy fit has finite overall and path SEs.
4617+
assert np.isfinite(res.overall_se)
4618+
assert res.path_effects is not None
4619+
any_finite_path = any(
4620+
np.isfinite(h["se"])
4621+
for e in res.path_effects.values()
4622+
for h in e["horizons"].values()
4623+
)
4624+
assert any_finite_path
4625+
4626+
# Force overall + event_study to NaN while leaving path_effects
4627+
# untouched — simulates the reversible-panel scenario where the
4628+
# overall IF is identically zero but the by_path subset IF is
4629+
# not.
4630+
res.overall_se = float("nan")
4631+
res.overall_t_stat = float("nan")
4632+
res.overall_p_value = float("nan")
4633+
res.overall_conf_int = (float("nan"), float("nan"))
4634+
if res.event_study_effects is not None:
4635+
for entry in res.event_study_effects.values():
4636+
entry["se"] = float("nan")
4637+
entry["t_stat"] = float("nan")
4638+
entry["p_value"] = float("nan")
4639+
entry["conf_int"] = (float("nan"), float("nan"))
4640+
4641+
summary_text = res.summary()
4642+
# Must NOT claim "non-finite SE on every target"
4643+
assert "produced non-finite SE on every target" not in summary_text, (
4644+
"Footer falsely claims all-target failure while path_effects "
4645+
"still has finite bootstrap SE. Summary tail:\n"
4646+
f"{summary_text[-400:]}"
4647+
)
4648+
# Must NOT claim "multiplier-bootstrap percentile inference"
4649+
# (overall_se is NaN so the headline inference is not bootstrap
4650+
# percentile).
4651+
assert "multiplier-bootstrap percentile inference" not in summary_text
4652+
# Must mention "per-path bootstrap inference is populated"
4653+
assert (
4654+
"per-path" in summary_text
4655+
and "bootstrap inference is populated" in summary_text
4656+
), (
4657+
"Footer must surface which targets retain finite bootstrap "
4658+
"inference when overall/event-study degenerates. Summary "
4659+
"tail:\n"
4660+
f"{summary_text[-400:]}"
4661+
)
4662+
45964663
def test_nan_contract_extends_to_overall_and_event_study_horizons(self):
45974664
"""
45984665
The bootstrap-contract NaN-on-invalid rule applies to every

0 commit comments

Comments
 (0)