Skip to content

Commit b23aca4

Browse files
igerberclaude
andcommitted
SpilloverDiD: address review polish — REGISTRY notes + rank_deficient_action guard
- REGISTRY: add "Note (anticipation shift)" documenting the public `anticipation: int` parameter's effect on both treatment and ring- membership clocks. Matches the implementation semantics at spillover.py:850-854 + 1441-1544, and mirrors TwoStageDiD's anticipation parameter convention. - REGISTRY: narrow the "correctness anchored on reduce-to-TWFE / reduce-to-TwoStageDiD limits" claim. Only the reduce-to-TWFE limit shipped (the 20-seed Gardner identity bit-identity test at TestSpilloverDiDNonStaggeredFEEquivalence). The reduce-to-TwoStageDiD limit was scoped during planning but not shipped — the Omega_0 stricter subsample requires fixture work to align with TwoStageDiD's {D_it = 0} subsample. Queued as a follow-up. - spillover.py: add `rank_deficient_action` constructor guard mirroring two_stage.py:149-153 and stacked_did.py. Bad values now fail at __init__ with a clear ValueError instead of deep inside solve_ols. - tests: new TestSpilloverDiDRankDeficientActionValidation class exercising 6 invalid values + 3 valid values. 148 tests pass (was 139); black + ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6f3ade4 commit b23aca4

3 files changed

Lines changed: 33 additions & 1 deletion

File tree

diff_diff/spillover.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,11 @@ def __init__(
899899
horizon_max: Optional[int] = None,
900900
rank_deficient_action: str = "warn",
901901
):
902+
if rank_deficient_action not in ("warn", "error", "silent"):
903+
raise ValueError(
904+
f"rank_deficient_action must be 'warn', 'error', or 'silent', "
905+
f"got '{rank_deficient_action}'"
906+
)
902907
self.rings = rings
903908
self.d_bar = d_bar
904909
self.vcov_type = vcov_type

docs/methodology/REGISTRY.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2969,7 +2969,9 @@ where `D_it` is the treatment indicator (1 if unit `i` is treated by time `t`) a
29692969

29702970
**Note:** Treated units have `d_it = 0` (their own location) and fall in Ring_1 geometrically, but the `(1 - D_it)` factor zeros their ring contribution. `n_units_ever_in_ring` counts include treated units in Ring_1 by construction. Under staggered timing this field counts each unit in EVERY ring it visits across periods (not a clean partition); under non-staggered timing it IS a partition.
29712971

2972-
**Note:** No published R/Stata software exists for the two-period ring estimator (Butts Eqs. 5-6). Correctness anchored on (a) synthetic-DGP Monte Carlo identification tests (50-seed default + 200-seed `@pytest.mark.slow` variant — both recover `tau_total` and `delta_1` within 0.02 absolute tolerance on Butts-Assumption-satisfying DGPs); (b) reduce-to-TWFE / reduce-to-TwoStageDiD limits when rings are far away; (c) Wave A's Conley sparse-vs-dense parity inherited via `solve_ols`.
2972+
**Note (anticipation shift):** The `anticipation: int` constructor kwarg (default 0) shifts BOTH the treatment and ring-membership clocks by `-anticipation` so the stage-1 Omega_0 subsample correctly excludes the `anticipation` pre-treatment periods (which would otherwise contaminate the FE estimation with anticipation effects). Concretely, the effective treatment indicator becomes `D'_it = 1{t >= first_treat - anticipation}` and ring membership uses the corresponding shifted "currently-treated" set when computing `d_it` for staggered timing. Stage 2 then estimates `tau_total` and `delta_j` net of the anticipation window. Mirrors `TwoStageDiD`'s `anticipation` parameter semantics — recommended use is a small integer (1-3 periods) when domain knowledge suggests treatment effects begin before formal onset (e.g., policy announcements). Setting `anticipation > 0` AND providing a `first_treat` column where `first_treat - anticipation < min(time)` for any unit will mark that unit as treated from the panel's first observation, with the same baseline-treated handling as if it were always-treated (warn-and-drop via the Omega_0 unit-level check above).
2973+
2974+
**Note:** No published R/Stata software exists for the two-period ring estimator (Butts Eqs. 5-6). Correctness anchored on (a) synthetic-DGP Monte Carlo identification tests (50-seed default + 200-seed `@pytest.mark.slow` variant — both recover `tau_total` and `delta_1` within 0.02 absolute tolerance on Butts-Assumption-satisfying DGPs); (b) reduce-to-TWFE limit (non-staggered, 20-seed deterministic bit-identity at `atol=1e-10` via `TestSpilloverDiDNonStaggeredFEEquivalence`); (c) Wave A's Conley sparse-vs-dense parity inherited via `solve_ols`. A reduce-to-`TwoStageDiD` limit was scoped during planning but not shipped in Wave B (the Omega_0 stricter subsample requires additional fixture work to align with `TwoStageDiD`'s `{D_it = 0}` subsample); queued as a follow-up alongside the Gardner GMM correction.
29732975

29742976
**Assumptions (Butts 2021):**
29752977

tests/test_spillover.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2072,6 +2072,31 @@ def test_hc2_paths_raise_not_implemented(self, vcov_type):
20722072
est.fit(df, outcome="y", unit="unit", time="time", treatment="D")
20732073

20742074

2075+
class TestSpilloverDiDRankDeficientActionValidation:
2076+
"""rank_deficient_action must be one of {warn, error, silent}.
2077+
Mirrors the sibling constructor guards at two_stage.py:149 and
2078+
stacked_did.py.
2079+
"""
2080+
2081+
@pytest.mark.parametrize("bad_value", ["raise", "ignore", "", "WARN", "Error", None])
2082+
def test_invalid_rank_deficient_action_raises_at_init(self, bad_value):
2083+
with pytest.raises(ValueError, match="rank_deficient_action must be"):
2084+
SpilloverDiD(
2085+
rings=[0.0, 100.0],
2086+
conley_coords=("lat", "lon"),
2087+
rank_deficient_action=bad_value,
2088+
)
2089+
2090+
@pytest.mark.parametrize("good_value", ["warn", "error", "silent"])
2091+
def test_valid_rank_deficient_action_accepted(self, good_value):
2092+
est = SpilloverDiD(
2093+
rings=[0.0, 100.0],
2094+
conley_coords=("lat", "lon"),
2095+
rank_deficient_action=good_value,
2096+
)
2097+
assert est.rank_deficient_action == good_value
2098+
2099+
20752100
# =============================================================================
20762101
# Codex review round-9 regression tests
20772102
# =============================================================================

0 commit comments

Comments
 (0)