Skip to content

Commit 0a2783a

Browse files
igerberclaude
andcommitted
Address PR #393 R0 P1: recompute path_cumulated_event_study post-bootstrap
CI reviewer flagged that the cumulated layer was built before the bootstrap propagation block, so when n_bootstrap > 0 it surfaced stale analytical SEs / t-stats / p-values / CIs while path_effects[path][l] had been overwritten with bootstrap stats — a public-surface inconsistency with the library-wide bootstrap contract. Fix: move the _compute_path_cumulated_event_study() call to AFTER the bootstrap propagation block at chaisemartin_dhaultfoeuille.py:3034-3081 (mirrors the global linear_trends_effects placement at :3405-3454). The helper reads path_effects[path]["horizons"][l]["se"] which by then is bootstrap-overwritten under n_bootstrap > 0, so cumulated SE becomes a running sum of bootstrap per-horizon SEs. Also addresses the P2 test-coverage gap with two new regressions in TestByPathTrendsLinear: - test_bootstrap_cumulated_uses_post_bootstrap_per_horizon_se: positive case asserting cumulated SE == running sum of post-bootstrap per- horizon SEs (rtol 1e-12). - test_bootstrap_cumulated_nan_consistent_when_n_bootstrap_one: n_bootstrap=1 case asserting cumulated SE / t_stat / p_value / conf_int are all NaN per the library-wide NaN-on-invalid contract. REGISTRY note updated with the post-bootstrap recomputation contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 88afbe4 commit 0a2783a

3 files changed

Lines changed: 142 additions & 23 deletions

File tree

diff_diff/chaisemartin_dhaultfoeuille.py

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2102,28 +2102,13 @@ def fit(
21022102
df_inference=_inference_df(_df_s_bp, resolved_survey),
21032103
set_ids=set_ids_arr,
21042104
)
2105-
# Per-path cumulated layer mirrors the global
2106-
# linear_trends_effects cumulation when trends_linear=True.
2107-
# path_effects[path]["horizons"][l] surfaces raw DID^{fd}_l;
2108-
# path_cumulated_event_study[path][l] surfaces the level
2109-
# effect delta_l = sum_{l'=1..l} DID^{fd}_{path, l'}.
2110-
if (
2111-
_is_trends_linear
2112-
and path_effects is not None
2113-
and len(path_effects) > 0
2114-
):
2115-
path_cumulated_event_study = _compute_path_cumulated_event_study(
2116-
D_mat=D_mat,
2117-
N_mat=N_mat,
2118-
first_switch_idx=first_switch_idx_arr,
2119-
switch_direction=switch_direction_arr,
2120-
L_max=L_max,
2121-
by_path=self.by_path,
2122-
multi_horizon_dids=multi_horizon_dids,
2123-
path_effects=path_effects,
2124-
alpha=self.alpha,
2125-
df_inference=_inference_df(_df_s_bp, resolved_survey),
2126-
)
2105+
# NOTE: per-path cumulated layer is computed AFTER the
2106+
# bootstrap propagation block below (search for
2107+
# `path_cumulated_event_study =`) so it reads the final
2108+
# post-bootstrap per-horizon SEs rather than the analytical
2109+
# ones that path_effects was just populated with. This
2110+
# mirrors the global `linear_trends_effects` cumulation
2111+
# which also runs after the event_study bootstrap propagation.
21272112

21282113
# Phase 2: placebos, normalized effects, cost-benefit delta
21292114
multi_horizon_placebos: Optional[Dict[int, Dict[str, Any]]] = None
@@ -3095,6 +3080,42 @@ def fit(
30953080
)
30963081
path_effects[path_key]["horizons"][l_h]["t_stat"] = np.nan
30973082

3083+
# Per-path cumulated layer (under trends_linear). Computed AFTER
3084+
# the bootstrap propagation block above so the cumulated SE / t /
3085+
# p / CI are derived from the FINAL post-bootstrap per-horizon
3086+
# path SEs rather than the analytical ones path_effects was
3087+
# initially populated with at fit-time. Mirrors the global
3088+
# `linear_trends_effects` placement at `:3405-3454` which also
3089+
# runs after the event_study bootstrap propagation. Honors the
3090+
# library-wide NaN-on-invalid bootstrap contract: any non-finite
3091+
# component SE in the running-sum upper bound yields a NaN
3092+
# cumulated SE / t / p / CI, regardless of whether the source
3093+
# was an analytical singularity or a non-finite bootstrap draw.
3094+
if (
3095+
self.by_path is not None
3096+
and _is_trends_linear
3097+
and L_max is not None
3098+
and L_max >= 1
3099+
and multi_horizon_dids is not None
3100+
and path_effects is not None
3101+
and len(path_effects) > 0
3102+
):
3103+
_df_s_bp_cum = _effective_df_survey(
3104+
resolved_survey, _replicate_n_valid_list
3105+
)
3106+
path_cumulated_event_study = _compute_path_cumulated_event_study(
3107+
D_mat=D_mat,
3108+
N_mat=N_mat,
3109+
first_switch_idx=first_switch_idx_arr,
3110+
switch_direction=switch_direction_arr,
3111+
L_max=L_max,
3112+
by_path=self.by_path,
3113+
multi_horizon_dids=multi_horizon_dids,
3114+
path_effects=path_effects,
3115+
alpha=self.alpha,
3116+
df_inference=_inference_df(_df_s_bp_cum, resolved_survey),
3117+
)
3118+
30983119
# Phase 3: propagate bootstrap results to per-path placebos
30993120
# (by_path + placebo). Sibling of the path_effects propagation
31003121
# block above. Library-wide NaN-on-invalid bootstrap contract:

0 commit comments

Comments
 (0)