Skip to content

Commit 4ecc6f4

Browse files
igerberclaude
andcommitted
Address PR #363 R2 review (2 P1)
R2 P1 (methodology): reject SurveyDesign(lonely_psu='adjust') with singleton strata in _sup_t_multiplier_bootstrap. The bootstrap helper pools singletons into a pseudo-stratum with NONZERO multipliers, but compute_survey_if_variance centers singleton PSU scores around the global mean — without the matching pseudo-stratum centering transform in the bootstrap, the simultaneous band target diverges from the analytical Binder-TSL variance. Clear NotImplementedError points users to lonely_psu='remove' (matches the 'remove' analytical target) or cband=False (skips bootstrap). 'remove' / 'certainty' continue to work unchanged. Deferred transform tracked for follow-up. R2 P1 (code quality): reject cluster= + weights/survey= on design='mass_point' on both static and event-study paths. The weighted path composes Binder-TSL variance via compute_survey_if_variance which was silently overriding the CR1 sandwich while result metadata still advertised vcov_type='cr1' and cluster_name=<col>. Clean NotImplementedError with a pointer to the two supported combinations: cluster= alone (unweighted CR1) or survey/weights alone (Binder-TSL). Combined cluster-robust survey inference requires a derivation not yet in scope. Regression tests (+4): - test_mass_point_survey_plus_cluster_rejected_static - test_mass_point_survey_plus_cluster_rejected_event_study - test_lonely_psu_adjust_with_singletons_rejected_on_cband - test_stratified_h1_sup_t_matches_analytical (H=1 quantile lock under n_strata=4, psu_per_unit: q=1.985, matches Phi^-1(0.975)) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 082368a commit 4ecc6f4

2 files changed

Lines changed: 218 additions & 0 deletions

File tree

diff_diff/had.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2062,6 +2062,45 @@ def _sup_t_multiplier_bootstrap(
20622062
)
20632063

20642064
if use_survey_bootstrap:
2065+
# Review R2 P1: lonely_psu="adjust" pools singleton strata into a
2066+
# pseudo-stratum with NONZERO multipliers in the bootstrap helper,
2067+
# but the analytical compute_survey_if_variance target for
2068+
# singletons is centered at the global mean of PSU scores. Since
2069+
# this PR's stratum-demean loop only matches the within-stratum
2070+
# Binder-TSL target (and skips singletons assuming zero
2071+
# contribution), pooled singleton multipliers would diverge from
2072+
# the analytical variance without an additional pseudo-stratum
2073+
# centering step. Reject with a clear pointer until the matching
2074+
# transform is derived; "remove" / "certainty" (singleton
2075+
# multipliers forced to zero) are fine.
2076+
_lonely = getattr(resolved_survey, "lonely_psu", "remove")
2077+
if _lonely == "adjust":
2078+
strata_arr = resolved_survey.strata
2079+
psu_arr = resolved_survey.psu
2080+
_has_singleton = False
2081+
if strata_arr is not None:
2082+
for h in np.unique(strata_arr):
2083+
mask_h = np.asarray(strata_arr) == h
2084+
if psu_arr is not None:
2085+
n_psu_h = int(np.unique(np.asarray(psu_arr)[mask_h]).shape[0])
2086+
else:
2087+
n_psu_h = int(mask_h.sum())
2088+
if n_psu_h < 2:
2089+
_has_singleton = True
2090+
break
2091+
if _has_singleton:
2092+
raise NotImplementedError(
2093+
"HeterogeneousAdoptionDiD event-study sup-t bootstrap "
2094+
"does not yet support SurveyDesign(lonely_psu='adjust') "
2095+
"with singleton strata: the bootstrap helper pools "
2096+
"singletons with nonzero multipliers while the "
2097+
"analytical Binder-TSL target centers singleton PSU "
2098+
"scores at the global mean, and the matching "
2099+
"pseudo-stratum centering transform has not been "
2100+
"implemented. Use lonely_psu='remove' (drops singleton "
2101+
"contributions; matches the 'remove' analytical target) "
2102+
"or pass cband=False to skip the simultaneous band."
2103+
)
20652104
psu_weights, psu_ids = generate_survey_multiplier_weights_batch(
20662105
n_bootstrap, resolved_survey, bootstrap_weights, rng
20672106
)
@@ -3260,6 +3299,25 @@ def fit(
32603299
vcov_label: Optional[str] = None
32613300
cluster_label: Optional[str] = None
32623301
elif resolved_design == "mass_point":
3302+
# Review R2 P1: mass-point + weights + cluster is a silent
3303+
# inference mismatch because the weighted path overrides the
3304+
# CR1 SE with Binder-TSL while result metadata still reports
3305+
# CR1. Reject the combination front-door until a combined
3306+
# cluster + survey variance is derived. Unweighted CR1
3307+
# continues to work unchanged; weighted pweight sandwich
3308+
# without cluster continues to work unchanged.
3309+
if cluster_arg is not None and weights_unit_full is not None:
3310+
raise NotImplementedError(
3311+
f"cluster={cluster_arg!r} + survey=/weights= on "
3312+
f"design='mass_point' is not yet supported: the "
3313+
f"weighted path composes Binder-TSL variance and "
3314+
f"would silently override the CR1 cluster-robust "
3315+
f"sandwich. Pass either cluster= alone (unweighted "
3316+
f"CR1) or survey=/weights= alone (weighted 2SLS "
3317+
f"pweight sandwich → Binder-TSL under survey=); "
3318+
f"combined cluster-robust survey inference is "
3319+
f"deferred to a follow-up PR."
3320+
)
32633321
if vcov_type_arg is None:
32643322
# Backward-compat: robust=True -> hc1, robust=False -> classical.
32653323
vcov_requested = "hc1" if robust_arg else "classical"
@@ -3858,6 +3916,22 @@ def _fit_event_study(
38583916
# ---- Extract cluster IDs on mass-point path only ----
38593917
cluster_arr: Optional[np.ndarray] = None
38603918
if resolved_design == "mass_point" and cluster_arg is not None:
3919+
# Review R2 P1: reject cluster= + weights/survey on
3920+
# mass-point (mirrors the static-path rejection) —
3921+
# the weighted path would compose Binder-TSL variance
3922+
# and silently override CR1 while result metadata still
3923+
# claims cluster-robust inference.
3924+
if weights_unit_full is not None:
3925+
raise NotImplementedError(
3926+
f"cluster={cluster_arg!r} + survey=/weights= on "
3927+
f"design='mass_point' (event-study) is not yet "
3928+
f"supported: the weighted path composes Binder-TSL "
3929+
f"variance and would silently override the CR1 "
3930+
f"cluster-robust sandwich. Pass either cluster= "
3931+
f"alone (unweighted CR1) or survey=/weights= alone; "
3932+
f"combined cluster-robust survey inference is "
3933+
f"deferred to a follow-up PR."
3934+
)
38613935
_, _, cluster_arr, _, _ = _aggregate_multi_period_first_differences(
38623936
data_filtered,
38633937
outcome_col,

tests/test_had.py

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4846,3 +4846,147 @@ def test_weights_nonrange_index_aligned_positionally(self):
48464846
)
48474847
np.testing.assert_allclose(r_range.att, r_shifted.att, atol=1e-12, rtol=1e-12)
48484848
np.testing.assert_allclose(r_range.se, r_shifted.se, atol=1e-12, rtol=1e-12)
4849+
4850+
def test_mass_point_survey_plus_cluster_rejected_static(self):
4851+
"""Review R2 P1: mass-point + (weights= or survey=) + cluster=
4852+
must raise NotImplementedError on the static path. Previously
4853+
the weighted path silently overrode the CR1 SE with Binder-TSL
4854+
while the result still reported vcov_type='cr1'."""
4855+
rng = np.random.default_rng(0)
4856+
G = 200
4857+
d = np.concatenate([np.full(40, 0.3), rng.uniform(0.3, 1.0, G - 40)])
4858+
rng.shuffle(d)
4859+
dy = 2.0 * d + 0.3 * rng.standard_normal(G)
4860+
panel = pd.DataFrame(
4861+
{
4862+
"unit": np.repeat(np.arange(G), 2),
4863+
"period": np.tile([1, 2], G),
4864+
"dose": np.column_stack([np.zeros(G), d]).ravel(),
4865+
"outcome": np.column_stack([np.zeros(G), dy]).ravel(),
4866+
"state": np.repeat(np.arange(G) // 20, 2),
4867+
}
4868+
)
4869+
with warnings.catch_warnings():
4870+
warnings.simplefilter("ignore", UserWarning)
4871+
est = HeterogeneousAdoptionDiD(design="mass_point", vcov_type="hc1", cluster="state")
4872+
with pytest.raises(NotImplementedError, match="cluster"):
4873+
est.fit(
4874+
panel,
4875+
"outcome",
4876+
"dose",
4877+
"period",
4878+
"unit",
4879+
weights=np.ones(panel.shape[0]),
4880+
)
4881+
4882+
def test_mass_point_survey_plus_cluster_rejected_event_study(self):
4883+
"""Review R2 P1 (event-study arm): same rejection must fire on
4884+
the multi-period dispatch."""
4885+
rng = np.random.default_rng(1)
4886+
G, T = 150, 4
4887+
d_mp = np.concatenate([np.full(30, 0.3), rng.uniform(0.3, 1.0, G - 30)])
4888+
rng.shuffle(d_mp)
4889+
rows = []
4890+
for t in range(T):
4891+
for g in range(G):
4892+
dose = d_mp[g] if t == T - 1 else 0.0
4893+
y = 0.2 * t + (2.0 * dose if t == T - 1 else 0.0) + 0.5 * rng.standard_normal()
4894+
rows.append((g, t, dose, y, g // 25))
4895+
panel = pd.DataFrame(rows, columns=["unit", "period", "dose", "outcome", "state"])
4896+
with warnings.catch_warnings():
4897+
warnings.simplefilter("ignore", UserWarning)
4898+
est = HeterogeneousAdoptionDiD(design="mass_point", vcov_type="hc1", cluster="state")
4899+
with pytest.raises(NotImplementedError, match="cluster"):
4900+
est.fit(
4901+
panel,
4902+
"outcome",
4903+
"dose",
4904+
"period",
4905+
"unit",
4906+
aggregate="event_study",
4907+
weights=np.ones(panel.shape[0]),
4908+
)
4909+
4910+
def test_lonely_psu_adjust_with_singletons_rejected_on_cband(self):
4911+
"""Review R2 P1: sup-t bootstrap rejects lonely_psu='adjust'
4912+
when there are singleton strata, because the bootstrap helper
4913+
pools singletons with nonzero multipliers but the analytical
4914+
target centers them at the global mean — mismatch."""
4915+
from diff_diff.had import _sup_t_multiplier_bootstrap
4916+
from diff_diff.survey import ResolvedSurveyDesign
4917+
4918+
rng = np.random.default_rng(0)
4919+
G = 80
4920+
# 3 strata, two with multiple PSUs, one singleton.
4921+
strata = np.array([1] * 30 + [2] * 30 + [3] * 20)
4922+
# PSUs: 10 in stratum 1, 10 in stratum 2, 1 in stratum 3 (singleton).
4923+
psu = np.concatenate(
4924+
[np.arange(10).repeat(3), (10 + np.arange(10)).repeat(3), np.full(20, 20)]
4925+
)
4926+
adjust_resolved = ResolvedSurveyDesign(
4927+
weights=np.ones(G),
4928+
weight_type="pweight",
4929+
strata=strata,
4930+
psu=psu,
4931+
fpc=None,
4932+
n_strata=3,
4933+
n_psu=21,
4934+
lonely_psu="adjust",
4935+
combined_weights=True,
4936+
mse=False,
4937+
)
4938+
psi = rng.standard_normal((G, 2))
4939+
with pytest.raises(NotImplementedError, match="lonely_psu='adjust'"):
4940+
_sup_t_multiplier_bootstrap(
4941+
psi,
4942+
np.zeros(2),
4943+
np.array([1.0, 1.0]),
4944+
adjust_resolved,
4945+
n_bootstrap=200,
4946+
alpha=0.05,
4947+
seed=0,
4948+
)
4949+
4950+
def test_stratified_h1_sup_t_matches_analytical(self):
4951+
"""Review R2 P1 coverage: stratum-centered H=1 bootstrap variance
4952+
matches the analytical Binder-TSL target (q ≈ 1.96 at H=1)."""
4953+
from diff_diff.had import _sup_t_multiplier_bootstrap
4954+
from diff_diff.survey import ResolvedSurveyDesign, compute_survey_if_variance
4955+
4956+
rng = np.random.default_rng(7)
4957+
G = 400
4958+
strata = np.repeat(np.arange(4), G // 4)
4959+
psu = np.arange(G)
4960+
resolved = ResolvedSurveyDesign(
4961+
weights=np.ones(G),
4962+
weight_type="pweight",
4963+
strata=strata,
4964+
psu=psu,
4965+
fpc=None,
4966+
n_strata=4,
4967+
n_psu=G,
4968+
lonely_psu="remove",
4969+
combined_weights=True,
4970+
mse=False,
4971+
)
4972+
psi = rng.standard_normal((G, 1))
4973+
V_analytical = compute_survey_if_variance(psi[:, 0], resolved)
4974+
se_analytical = np.sqrt(V_analytical)
4975+
q, _, _, _ = _sup_t_multiplier_bootstrap(
4976+
psi,
4977+
np.zeros(1),
4978+
np.array([se_analytical]),
4979+
resolved,
4980+
n_bootstrap=5000,
4981+
alpha=0.05,
4982+
seed=42,
4983+
)
4984+
# At H=1 the sup collapses to the marginal; with stratum-
4985+
# centered + small-sample-corrected perturbations the bootstrap
4986+
# distribution is ~ N(0, 1), so q → Phi^-1(0.975) = 1.96.
4987+
# B=5000 MC noise on the tail quantile is ~0.03-0.05.
4988+
assert abs(q - 1.96) < 0.15, (
4989+
f"Stratified H=1 sup-t should match Normal quantile 1.96 up to "
4990+
f"MC noise; got q={q:.4f}. Likely a stratum-centering bug in "
4991+
f"_sup_t_multiplier_bootstrap."
4992+
)

0 commit comments

Comments
 (0)