Skip to content

Commit a29a8fa

Browse files
igerberclaude
andcommitted
Address CI AI review round 3: default boundary=0 classification, rank test
P1 (methodology): The boundary=0 default path now classifies inputs per the REGISTRY-planned design="auto" rule. Previously the mass-point guard only fired for boundary>0, so a dataset with d.min()>0 and mass at d.min() would silently pass through the default-boundary path as Design 1'. The new rule: - boundary=0 with min(d) < 0.01 * median(d): Design 1' accepted (support infimum effectively at 0). - boundary=0 with min(d) >= 0.01 * median(d): * modal fraction at min(d) > 2%: mass-point design -> raise NotImplementedError pointing to the 2SLS / Phase 2 path. * otherwise: ambiguous -> raise ValueError asking the caller to pass boundary=d.min() for the Design 1 continuous- near-d_lower path. - boundary>0 path unchanged (mass-point check already in place). Removed the stale test_boundary_below_min_d_accepted: it used U(0.01, 1) data which doesn't satisfy Design 1' under the stricter rule. Replaced with three targeted tests: - test_boundary_zero_design_1_prime_accepted: U(0, 1) passes. - test_boundary_zero_with_positive_d_min_rejected: U(0.5, 1) raises "Ambiguous design". - test_boundary_zero_with_d_min_mass_point_rejected: mass at 0.1 with boundary=0 raises mass-point NotImplementedError. P3 (tests): Added test_full_stack_rank_deficient_raises_valueerror driving a 3-distinct-value dataset through both lpbwselect_mse_dpi and the public wrapper; both must raise ValueError (or NotImplementedError), never LinAlgError or IndexError. 172 tests pass (up from 169). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 03532fc commit a29a8fa

2 files changed

Lines changed: 107 additions & 18 deletions

File tree

diff_diff/local_linear.py

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -633,21 +633,24 @@ def mse_optimal_bandwidth(
633633
f"separately parity-tested against nprobust."
634634
)
635635

636-
# Mass-point design check (REGISTRY.md plans `>2%` modal-min rule).
637-
# If d_lower > 0 and the distribution bunches at d_lower, the paper
638-
# (de Chaisemartin et al. 2026 Section 3.2.4) prescribes the 2SLS
639-
# sample-average path, NOT the nonparametric CCF local-polynomial
640-
# path. Detect bunching and redirect the caller.
636+
# Design classification (REGISTRY.md Phase 2 design="auto" rule
637+
# plus paper Section 3.2.4 mass-point redirection):
641638
#
642-
# We only flag when boundary > 0 (Design 1 continuous-near-d_lower
643-
# vs Design 1 mass-point). For boundary = 0 (Design 1' or "untreated
644-
# units present" subcase), the paper accepts nonparametric even with
645-
# mass at 0 (Garrett et al. 2020 application with 12/2954 at 0).
639+
# Design 1' <=> d_lower = inf supp(D_2) = 0
640+
# Design 1 masspt <=> d_lower > 0, P(D_2 = d_lower) > 0
641+
# Design 1 cont <=> d_lower > 0, D_2 continuous near d_lower
642+
#
643+
# The CCF nonparametric selector is appropriate for Design 1' and
644+
# Design 1 continuous-near-d_lower. Mass-point Design 1 requires a
645+
# 2SLS sample-average estimator (Phase 2, not Phase 1b).
646+
_MASS_POINT_THRESHOLD = 0.02 # REGISTRY rule: > 2% modal-min
647+
eps_eq = 1e-12 * max(1.0, abs(d_min))
648+
at_d_min_mask = np.abs(d - d_min) <= eps_eq
649+
modal_fraction = float(np.mean(at_d_min_mask))
650+
646651
if boundary > _boundary_tol:
647-
eps_eq = 1e-12 * max(1.0, abs(d_min))
648-
at_boundary_mask = np.abs(d - d_min) <= eps_eq
649-
modal_fraction = float(np.mean(at_boundary_mask))
650-
_MASS_POINT_THRESHOLD = 0.02 # REGISTRY rule: > 2% modal-min
652+
# User supplied boundary = d_min > 0 explicitly. This is
653+
# Design 1; distinguish mass-point vs continuous-near-d_lower.
651654
if modal_fraction > _MASS_POINT_THRESHOLD:
652655
raise NotImplementedError(
653656
f"Detected mass-point design: the lower boundary "
@@ -662,6 +665,42 @@ def mse_optimal_bandwidth(
662665
f"near-d_lower designs (modal fraction <= "
663666
f"{_MASS_POINT_THRESHOLD:.2f}), this wrapper is applicable."
664667
)
668+
else:
669+
# User passed boundary <= 0 (default). Intent is Design 1'.
670+
# Apply the REGISTRY's `min(d) < 0.01 * median(d)` rule: only
671+
# accept when the support infimum is effectively 0 relative to
672+
# the data scale. Otherwise force the user to disambiguate.
673+
d_median = float(np.median(d))
674+
_DESIGN_1_PRIME_RATIO = 0.01 # REGISTRY: min(d)/median(d) < 1%
675+
# Guard against pathological all-zero or all-negative median.
676+
effective_threshold = _DESIGN_1_PRIME_RATIO * max(abs(d_median), 1e-12)
677+
if d_min > effective_threshold:
678+
if modal_fraction > _MASS_POINT_THRESHOLD:
679+
# Mass-point design dressed as Design 1': same
680+
# NotImplementedError as the boundary>0 branch.
681+
raise NotImplementedError(
682+
f"Detected mass-point design at d.min()={d_min!r} "
683+
f"(modal fraction {modal_fraction:.4f} > "
684+
f"{_MASS_POINT_THRESHOLD:.2f}), but boundary=0 "
685+
f"implies Design 1' intent. Either: (a) pass "
686+
f"boundary=d.min() to make the mass-point "
687+
f"classification explicit (which will then raise "
688+
f"NotImplementedError directing to the 2SLS path "
689+
f"per de Chaisemartin et al. 2026 Section 3.2.4), "
690+
f"or (b) reconsider whether the data is truly "
691+
f"Design 1' (support at 0)."
692+
)
693+
raise ValueError(
694+
f"Ambiguous design: boundary=0 but d.min()={d_min!r} "
695+
f"exceeds the Design 1' threshold of "
696+
f"0.01 * median(d) = {effective_threshold!r}. This "
697+
f"dataset does not satisfy Design 1' (support infimum "
698+
f"at 0). Either: (a) pass boundary=d.min() for the "
699+
f"Design 1 continuous-near-d_lower path, or (b) verify "
700+
f"the data truly has support at 0 (in which case "
701+
f"d.min() would be much closer to zero relative to the "
702+
f"data scale)."
703+
)
665704

666705
# Defer heavy import to call time to avoid import-cycle risk.
667706
from diff_diff._nprobust_port import lpbwselect_mse_dpi

tests/test_bandwidth_selector.py

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -312,16 +312,41 @@ def test_boundary_equal_to_min_d_accepted(self):
312312
assert np.isfinite(h)
313313
assert h > 0.0
314314

315-
def test_boundary_below_min_d_accepted(self):
316-
"""Design 1' uses boundary = 0 when all D_2 > 0; boundary
317-
below d.min() must pass."""
315+
def test_boundary_zero_design_1_prime_accepted(self):
316+
"""Design 1' data: d.min() effectively 0 relative to median,
317+
so boundary=0 passes the REGISTRY 1%-of-median rule."""
318318
rng = np.random.default_rng(20260419)
319-
d = rng.uniform(0.01, 1.0, size=1500) # d.min() > 0
320-
y = d + d**2 + rng.normal(0, 0.5, size=1500)
319+
# d.min() ~ 5e-4 << 0.01 * median(d) ~ 5e-3
320+
d = rng.uniform(0.0, 1.0, size=3000)
321+
y = d + d**2 + rng.normal(0, 0.5, size=3000)
321322
h = mse_optimal_bandwidth(d, y, boundary=0.0)
322323
assert np.isfinite(h)
323324
assert h > 0.0
324325

326+
def test_boundary_zero_with_positive_d_min_rejected(self):
327+
"""Default boundary=0 with d.min() substantially positive (>1%
328+
of median) is not Design 1'; force the caller to pass
329+
boundary=d.min() instead of silently misclassifying."""
330+
rng = np.random.default_rng(2026)
331+
# d.min() ~ 0.5, median ~ 0.75 -> ratio 0.67 >> 0.01.
332+
d = rng.uniform(0.5, 1.0, size=1500)
333+
y = d + rng.normal(0, 0.3, size=1500)
334+
with pytest.raises(ValueError, match="Ambiguous design"):
335+
mse_optimal_bandwidth(d, y, boundary=0.0)
336+
337+
def test_boundary_zero_with_d_min_mass_point_rejected(self):
338+
"""boundary=0 default path with d.min() > 0 AND mass at d.min()
339+
must also be rejected, pointing to the 2SLS redirection."""
340+
rng = np.random.default_rng(2026)
341+
n_mass = 300 # 15% at 0.1
342+
n_cont = 1700
343+
d_mass = np.full(n_mass, 0.1)
344+
d_cont = rng.uniform(0.1, 1.0, size=n_cont)
345+
d = np.concatenate([d_mass, d_cont])
346+
y = d + rng.normal(0, 0.5, size=d.size)
347+
with pytest.raises(NotImplementedError, match="mass-point"):
348+
mse_optimal_bandwidth(d, y, boundary=0.0)
349+
325350
def test_mass_point_design_rejected(self):
326351
"""Design 1 mass-point case (boundary > 0, modal fraction > 2%)
327352
must be rejected with NotImplementedError pointing to 2SLS."""
@@ -369,6 +394,31 @@ def test_rank_deficient_design_raises_valueerror(self):
369394
with pytest.raises(ValueError, match="qrXXinv"):
370395
qrXXinv(X)
371396

397+
def test_full_stack_rank_deficient_raises_valueerror(self):
398+
"""End-to-end rank-deficiency integration test: a sample with
399+
enough rows but only a handful of distinct in-window d values
400+
drives rank-deficient X'X at some DPI stage. The public wrapper
401+
must surface a clear ValueError (either from qrXXinv's Cholesky
402+
guard or from the stage-count guards), not IndexError or
403+
LinAlgError.
404+
"""
405+
from diff_diff._nprobust_port import lpbwselect_mse_dpi
406+
407+
# Only 3 distinct d values; each pilot stage's design matrix
408+
# will have at most 3 independent rows, but the B2 fit needs
409+
# o_B+2 = 5 or 6 columns -> rank deficient.
410+
d = np.repeat([0.1, 0.2, 0.3], 50).astype(np.float64)
411+
rng = np.random.default_rng(2026)
412+
y = d + rng.normal(0, 0.1, size=d.size)
413+
with pytest.raises(ValueError):
414+
lpbwselect_mse_dpi(y, d, eval_point=0.0, bwcheck=None)
415+
# Same through the public wrapper (boundary=0 is fine since
416+
# d.min()=0.1 triggers either ambiguous-design or rank-deficient
417+
# rejection; we only care that it is a ValueError and not an
418+
# opaque linear-algebra crash).
419+
with pytest.raises((ValueError, NotImplementedError)):
420+
mse_optimal_bandwidth(d, y)
421+
372422

373423
class TestKernelDispatch:
374424
"""Different kernels produce different bandwidths."""

0 commit comments

Comments
 (0)