Skip to content

Commit 79a01ab

Browse files
igerberclaude
andcommitted
Fix CI review Round 7: suppress joiner/leaver for all L_max>=1, no-switcher guard
P1: Suppress joiner/leaver decomposition for ALL L_max >= 1 (not just non-binary). The decomposition is a per-period DID_M concept that can differ from the per-group DID_1 estimand on mixed panels. P1: Add no-switcher guard after multi-horizon computation - raise ValueError if N_l == 0 at horizon 1 (catches constant-treatment non-binary panels). P3: Update REGISTRY SE parity tolerance note (10%/15% for multi-horizon, 5% for single-horizon). Fix stale Step 12c comment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 46273b6 commit 79a01ab

2 files changed

Lines changed: 26 additions & 13 deletions

File tree

diff_diff/chaisemartin_dhaultfoeuille.py

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,7 +1048,7 @@ def fit(
10481048
)
10491049

10501050
# ------------------------------------------------------------------
1051-
# Step 12c: Multi-horizon computation (Phase 2, only when L_max>=2)
1051+
# Step 12c: Multi-horizon per-group computation (L_max >= 1)
10521052
# ------------------------------------------------------------------
10531053
multi_horizon_dids: Optional[Dict[int, Dict[str, Any]]] = None
10541054
multi_horizon_if: Optional[Dict[int, np.ndarray]] = None
@@ -1080,6 +1080,15 @@ def fit(
10801080
stacklevel=2,
10811081
)
10821082

1083+
# Guard: if no eligible switchers at horizon 1 (e.g., all
1084+
# groups have constant treatment), raise ValueError.
1085+
if 1 in multi_horizon_dids and multi_horizon_dids[1]["N_l"] == 0:
1086+
raise ValueError(
1087+
"No switching groups found at horizon 1 after filtering. "
1088+
"dCDH requires at least one group whose treatment changes "
1089+
"from the baseline period."
1090+
)
1091+
10831092
multi_horizon_if = _compute_per_group_if_multi_horizon(
10841093
D_mat=D_mat,
10851094
Y_mat=Y_mat,
@@ -1831,28 +1840,32 @@ def fit(
18311840
twfe_sigma_fe = twfe_diagnostic_payload.sigma_fe
18321841
twfe_beta_fe = twfe_diagnostic_payload.beta_fe
18331842

1834-
# When L_max >= 1 on non-binary data, the binary-only metadata
1835-
# (N_S, joiner/leaver counts, n_treated_obs) doesn't match the
1836-
# per-group DID_1 estimand. Use per-group metadata instead and
1837-
# suppress the joiner/leaver decomposition.
1843+
# When L_max >= 1, the overall estimand is per-group DID_1
1844+
# (not per-period DID_M). The joiner/leaver decomposition is a
1845+
# per-period DID_M concept and can differ from DID_1 on mixed
1846+
# panels, so it's suppressed for all L_max >= 1 cases. N_S and
1847+
# n_treated_obs are updated from the per-group path.
18381848
effective_N_S = N_S
18391849
effective_n_treated = n_treated_obs_post
18401850
effective_joiners_available = joiners_available
18411851
effective_leavers_available = leavers_available
18421852
if (
1843-
not is_binary
1844-
and L_max is not None
1853+
L_max is not None
18451854
and L_max >= 1
18461855
and multi_horizon_dids is not None
18471856
and 1 in multi_horizon_dids
18481857
):
18491858
# Use horizon-1 eligible switcher count as the effective N_S
18501859
effective_N_S = multi_horizon_dids[1]["N_l"]
1851-
# Count all observations where treatment differs from baseline
1852-
effective_n_treated = int(
1853-
N_mat[D_mat != D_mat[:, 0:1]].sum()
1854-
) if D_mat.shape[1] > 1 else 0
1855-
# Suppress joiner/leaver decomposition for non-binary
1860+
if not is_binary:
1861+
# For non-binary: count all observations where treatment
1862+
# differs from baseline
1863+
effective_n_treated = int(
1864+
N_mat[D_mat != D_mat[:, 0:1]].sum()
1865+
) if D_mat.shape[1] > 1 else 0
1866+
# Suppress joiner/leaver decomposition for all L_max >= 1
1867+
# (the decomposition is a per-period DID_M concept, not
1868+
# applicable to the per-group DID_1 estimand)
18561869
effective_joiners_available = False
18571870
effective_leavers_available = False
18581871

docs/methodology/REGISTRY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ Alternative: Multiplier bootstrap clustered at group via the `n_bootstrap` param
601601

602602
- **Note:** Groups whose baseline treatment value `D_{g,1}` is unique in the post-drop panel (not shared by any other group) are excluded from the **variance computation only** per footnote 15 of the dynamic companion paper. They have no cohort peer for the cohort-recentered plug-in formula. They are **retained in the point-estimate sample** as period-based stable controls (Python's documented period-vs-cohort interpretation). The dropped count is stored on `results.n_groups_dropped_singleton_baseline`, a warning lists example group IDs, and the warning text explicitly states "VARIANCE computation only" so users know the filter does not change `DID_M`.
603603

604-
- **Note (deviation from R DIDmultiplegtDYN):** Python uses **period-based** stable-control sets — `stable_0(t)` is any cell with `D_{g,t-1} = D_{g,t} = 0` regardless of baseline `D_{g,1}`, and similarly for `stable_1(t)`. R `DIDmultiplegtDYN` uses **cohort-based** stable-control sets that additionally require `D_{g,1}` to match the side. Python's definition matches the AER 2020 Theorem 3 cell-count notation `N_{0,0,t}` and `N_{1,1,t}` literally; R's definition matches the dynamic companion paper's cohort `(D_{g,1}, F_g, S_g)` framework. The two definitions agree exactly on (a) panels containing only joiners, (b) panels containing only leavers, (c) the hand-calculable 4-group worked example, or (d) any panel where no joiner's post-switch state overlaps a period when leavers are switching. They disagree by O(1%) on the **point estimate** when both joiners and leavers exist AND some joiners' post-switch cells could serve as leavers' controls (or vice versa). After the Round 2 fix that implemented the full `Lambda^G_{g,l=1}` influence function, the **standard error** parity gap on pure-direction scenarios narrowed from ~18% to ~3%. The R parity tests in `tests/test_chaisemartin_dhaultfoeuille_parity.py` use a tight `1e-4` tolerance for pure-direction point estimates, a 5% rtol for pure-direction SEs, and a 2.5% tolerance for mixed-direction point estimates (with the SE check skipped on mixed scenarios because the period-vs-cohort point-estimate deviation cascades into the variance).
604+
- **Note (deviation from R DIDmultiplegtDYN):** Python uses **period-based** stable-control sets — `stable_0(t)` is any cell with `D_{g,t-1} = D_{g,t} = 0` regardless of baseline `D_{g,1}`, and similarly for `stable_1(t)`. R `DIDmultiplegtDYN` uses **cohort-based** stable-control sets that additionally require `D_{g,1}` to match the side. Python's definition matches the AER 2020 Theorem 3 cell-count notation `N_{0,0,t}` and `N_{1,1,t}` literally; R's definition matches the dynamic companion paper's cohort `(D_{g,1}, F_g, S_g)` framework. The two definitions agree exactly on (a) panels containing only joiners, (b) panels containing only leavers, (c) the hand-calculable 4-group worked example, or (d) any panel where no joiner's post-switch state overlaps a period when leavers are switching. They disagree by O(1%) on the **point estimate** when both joiners and leavers exist AND some joiners' post-switch cells could serve as leavers' controls (or vice versa). After the Round 2 fix that implemented the full `Lambda^G_{g,l=1}` influence function, the **standard error** parity gap on pure-direction scenarios narrowed from ~18% to ~3%. The R parity tests in `tests/test_chaisemartin_dhaultfoeuille_parity.py` use a tight `1e-4` tolerance for pure-direction point estimates, 10% rtol for multi-horizon SEs (15% for L_max=5 long panels where the cell-count weighting deviation compounds), 5% rtol for single-horizon SEs, and a 2.5% tolerance for mixed-direction point estimates (with the SE check skipped on mixed scenarios because the period-vs-cohort point-estimate deviation cascades into the variance).
605605

606606
- **Note (deviation from R DIDmultiplegtDYN):** Phase 1 requires panels with a **balanced baseline** (every group observed at the first global period) and **no interior period gaps**. The Step 5b validation in `fit()` enforces this contract: groups missing the baseline raise `ValueError`; groups with interior gaps are dropped with a `UserWarning`; groups with **terminal missingness** (early exit / right-censoring — observed at the baseline but missing one or more later periods) are retained and contribute from their observed periods only. R `DIDmultiplegtDYN` accepts unbalanced panels with documented missing-treatment-before-first-switch handling. Python's restriction is a Phase 1 limitation: the cohort enumeration uses `D_{g,1}` as the canonical baseline (so the baseline observation must exist) and the first-switch detection walks adjacent observed periods (so interior gaps create ambiguous transition counts). Terminal missingness is supported because the per-period `present = (N_mat[:, t] > 0) & (N_mat[:, t-1] > 0)` guard appears at three sites in the variance computation (`_compute_per_period_dids`, `_compute_full_per_group_contributions`, `_compute_cohort_recentered_inputs`) and cleanly masks out missing transitions without propagating NaN into the arithmetic. **Workaround for unbalanced panels:** pre-process your data to back-fill the baseline (or drop late-entry groups before fitting), or use R `DIDmultiplegtDYN` until a future phase lifts the restriction. The Step 5b `ValueError` and `UserWarning` messages name the offending group IDs so you can locate them quickly.
607607

0 commit comments

Comments
 (0)