Skip to content

Commit d0d3237

Browse files
authored
feat(validation): family-wide anticipation domain validation (M-144) (#771)
1 parent e9d1259 commit d0d3237

24 files changed

Lines changed: 642 additions & 97 deletions

CHANGELOG.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
116116
expression), pointing at `full_model_window=True` as the mroi route.
117117

118118
### Changed
119+
- **Family-wide `anticipation` domain validation ([M-144], landing at 4.0 —
120+
the locked ladder's next release, with no warn-then-error window per the
121+
M-096/M-142 precedent that validation tightenings ship immediately;
122+
retires the TODO "library-wide anticipation domain validation" row).** All
123+
nine anticipation-taking estimators (`CallawaySantAnna`, `SunAbraham`,
124+
`ImputationDiD`, `TwoStageDiD`, `StackedDiD`, `ContinuousDiD`,
125+
`EfficientDiD`, `WooldridgeDiD`, `SpilloverDiD`) now validate
126+
`anticipation` at construction via the shared
127+
`utils.validate_anticipation` (non-negative integer; `bool` rejected;
128+
`set_params` transactional) AND re-check it on the fit path — the uniform
129+
direct-mutation defense, in the assignment form that also normalizes
130+
numpy scalars to built-in `int`. Previously seven of the nine accepted
131+
anything, and an out-of-domain window silently changed the ESTIMAND:
132+
measured, `CallawaySantAnna(anticipation=-1)` moved the overall ATT by
133+
−85% and flipped its sign under `control_group="not_yet_treated"`;
134+
`anticipation=True` fit bit-identically to `1` (a silent one-period
135+
window); `SunAbraham(anticipation=1.5)` returned `att=nan` without
136+
raising. `0` stays legal. The one break of previously-CORRECT code:
137+
whole-valued floats (`anticipation=1.0`, `np.float64(1.0)`) previously
138+
fit bit-identically to their integer value on
139+
CS/SunAbraham/ImputationDiD/TwoStageDiD/EfficientDiD/WooldridgeDiD and
140+
now raise — use the int (StackedDiD already crashed on floats via an
141+
incidental `range()` TypeError, now a clear constructor `ValueError`;
142+
ContinuousDiD's float behavior was fixture-dependent). Also visible:
143+
accepted numpy integers are retyped — the public `anticipation`
144+
attribute and `get_params()["anticipation"]` are now always built-in
145+
`int`, not a numpy scalar; `WooldridgeDiD`'s message text changed to the
146+
shared wording, its `None`/str raw `TypeError` became `ValueError`, and
147+
its constructor error ordering moved (bad
148+
`bootstrap_weights`/`vcov_type`/`df_convention` now report before a bad
149+
`anticipation`); `SpilloverDiD`'s raise moved from fit to construction
150+
(the fit-time re-check is retained, ordered before the ref-period
151+
arithmetic), and its negative-int message dropped the `(type ...)`
152+
suffix (shared text). `EfficientDiD.hausman_pretest` normalizes its own
153+
`anticipation` argument (an unsigned numpy scalar previously wrapped its
154+
event-time arithmetic and silently degraded the pretest to an all-NaN
155+
inconclusive result). The deprecated `StaggeredTripleDifference` stays
156+
construction-permissive by design (fit-validated via the shared engine).
157+
Both LLM guides note the domain; policy suite:
158+
`tests/test_anticipation_policy.py`.
119159
- **DiagnosticReport's event-study-gated checks now consume the post-fit
120160
`results.aggregate('event_study')` surface** (the 3.9 M-020 family;
121161
retires the TODO "diagnostic_report ES-gated checks" row): on a modern

TODO.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ Related tracking surfaces:
2424
| Post-fit `aggregate()` for the staggered DDD container: `StaggeredTripleDiffResults` carries no `AggregationMixin`, which is why the phase-3(b) merge had to carry fit-time `aggregate=`/`balance_e=` onto the surviving `TripleDifference` (rows M-140/M-141) as the ONE documented exception to the section-6 aggregate-postfit program. Porting the container onto the M-122 aggregation contract retires both rows; note the bootstrapped-fit recompute levels will need draw retention or a fail-closed relay, the same problem tracked for CS/EfficientDiD/ImputationDiD. Until it lands, the DDD docs deliberately keep teaching the fit-time kwarg (the canonical route there) | `diff_diff/staggered_triple_diff_results.py`, `diff_diff/aggregation.py`, `docs/api/triple_diff.rst`, `docs/tutorials/08_triple_diff.ipynb` | 3(b) | Heavy | Medium |
2525
| Staggered-DDD power support: `simulate_power`/`simulate_mde`/`simulate_sample_size` now REJECT a staggered-configured `TripleDifference` (both registered DDD generators emit 2x2x2 data and fit with `(group, partition, post)`, so a staggered config would be simulated under the wrong design). Support needs a staggered DDD DGP profile plus fit-kwargs builder, and a decision on whether the mode is selected by profile or by the estimator's own config | `diff_diff/power.py` | 3(b) | Mid | Low |
2626
| Bootstrap-`seed` provenance on multiplier-bootstrap results containers: neither `StaggeredTripleDiffResults` nor `CallawaySantAnnaResults` carries the `seed` that generated its bootstrap SEs / p-values / sup-t bands, so a serialized result cannot report the random configuration behind its inference. NOT a 3(b) regression - `seed` reaches the engine and `get_params()` correctly (same seed reproduces the SE bit-exactly, a different seed moves it), the gap is results-object observability only, it predates the merge, and both containers inherit it from the shared `CallawaySantAnnaBootstrapMixin`. Add `seed` (and consider `n_bootstrap`/`bootstrap_weights`/`cband`) to BOTH containers plus `to_dict()`, with seeded and unseeded pins; sequence it with the M-014 container unification rather than schema-changing one container mid-merge. Precedent for exposing it: `ContinuousDiDResults`, `EfficientDiDResults`, `SyntheticDiDResults` already do | `diff_diff/staggered_triple_diff_results.py`, `diff_diff/staggered_results.py` | 3(b) | Quick | Low |
27-
| Library-wide `anticipation` domain validation: `TripleDifference` now rejects non-integral / negative / `bool` windows at construction (phase 3(b)) because the value feeds BOTH the base-period rule and the not-yet-treated threshold, so `anticipation=-1` silently makes the universal base period `g` (already treated) and admits cohorts treated at the evaluation period as clean controls. Only `spillover.py` and `wooldridge.py` validate it today (and neither rejects `bool`, which coerces to a silent one-period window); `CallawaySantAnna`, `SunAbraham`, `ImputationDiD`, `TwoStageDiD`, `StackedDiD`, `ContinuousDiD`, `EfficientDiD` and the deprecated `StaggeredTripleDifference` do not. The shared validator now EXISTS - `utils.validate_anticipation`, adopted by `TripleDifference.__init__` and by the staggered engine (so `StaggeredTripleDifference` fails closed at fit too); aligning the remaining seven estimators is a matter of calling it from each constructor | `diff_diff/staggered.py`, `diff_diff/sun_abraham.py`, `diff_diff/imputation.py`, `diff_diff/two_stage.py`, `diff_diff/stacked_did.py`, `diff_diff/continuous_did.py`, `diff_diff/efficient_did.py`, `diff_diff/spillover.py`, `diff_diff/wooldridge.py` | 3(b) | Mid | Medium |
2827
| `ContinuousDiD.pscore_trim` still validates `0.0 <= x < 0.5`, i.e. it admits `0`, while `TripleDifference` tightened to `0 < x < 0.5` in phase 3(b) (row M-142) on the grounds that `trim=0` disables the `np.clip(pscore, trim, 1-trim)` overlap guard keeping the `1/(1-p)` weights finite. The same argument applies to ContinuousDiD; aligning it was out of scope for a DDD merge and is recorded in the REGISTRY staggered-mode Note rather than left as silent drift. `TripleDifference` additionally gained a TYPE guard in 3(b) (reject bool/non-real-scalar/non-finite BEFORE the range comparison) because a bare `0 < x < 0.5` raises an incidental `TypeError` on `None`/str/complex/list, an ambiguous-truth error on a multi-element array, and silently ACCEPTS a 1-element array as the parameter; `ContinuousDiD`'s `np.isfinite(self.pscore_trim) and ...` has the same hole. Aligning both is one change - promote the guard to a shared `utils.validate_pscore_trim(value, *, allow_zero)` alongside `validate_n_bootstrap` rather than copying it | `diff_diff/continuous_did.py`, `diff_diff/utils.py` | 3(b) | Quick | Low |
2928
| Staggered-mode cluster-robust ANALYTICAL SEs: `cluster=` raises in `TripleDifference`'s staggered mode (and is accepted-then-ignored on the deprecated class), so clustered inference there is bootstrap-only. Implementing a clustered analytical path for the GMM-combined influence function would let the raise become a real lane | `diff_diff/_staggered_triple_diff_engine.py` | 3(b) | Heavy | Low |
3029
| diagnostic_report admission for `EventStudyResults` surfaces (the TWFE event-study mode + `aggregate('event_study')` containers): DiagnosticReport/BusinessReport now REJECT the surface explicitly (Phase 3(a); previously a silent zero-check report / all-null headline) and practitioner_next_steps serves the generic fall-through - admission needs source-aware routing (the type-name-keyed `_APPLICABILITY`/`_HANDLERS` registries cannot discriminate the unified container's producers) and a scalar-vs-per-period headline design; MPD-native results received {parallel_trends, pretrends_power, sensitivity, bacon, design_effect} | `diff_diff/diagnostic_report.py`, `diff_diff/business_report.py`, `diff_diff/practitioner.py` | 3(a) | Mid | Medium |

diff_diff/_staggered_triple_diff_engine.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,13 @@ class _StaggeredTripleDiffEngineMixin:
6464
supply the constructor attributes and the CS aggregation/bootstrap mixins
6565
this core calls. The annotations below exist because mypy type-checks this
6666
class independently of its hosts (`attr-defined` is not disabled) - they are
67-
declarations, never assignments.
67+
declarations, never assignments — with ONE exception: `anticipation` is
68+
re-assigned by the core's fit-time re-validation (the mutation-defense
69+
re-check normalizes it to a Python int; see `_fit_staggered_core`).
6870
"""
6971

70-
# Constructor attributes read from the host class.
72+
# Constructor attributes read from the host class. (`anticipation` is
73+
# additionally RE-ASSIGNED at fit — the validate-and-normalize re-check.)
7174
estimation_method: str
7275
control_group: str
7376
alpha: float
@@ -178,8 +181,10 @@ def _fit_staggered_core(
178181
# the deprecated StaggeredTripleDifference (whose 3.x API SHAPE is
179182
# frozen through removal - that freeze was never a licence to emit
180183
# silently-biased numbers) and direct attribute mutation on either
181-
# class, which bypasses __init__ and set_params alike.
182-
validate_anticipation(self.anticipation)
184+
# class, which bypasses __init__ and set_params alike. The
185+
# assignment form also normalizes a numpy scalar to a Python int
186+
# before any `g - 1 - anticipation` arithmetic can overflow.
187+
self.anticipation = validate_anticipation(self.anticipation)
183188
from diff_diff.survey import (
184189
_resolve_survey_for_fit,
185190
_validate_unit_constant_survey,

diff_diff/continuous_did.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
build_unit_first_row_index,
4646
compute_survey_vcov,
4747
)
48-
from diff_diff.utils import safe_inference, validate_n_bootstrap
48+
from diff_diff.utils import safe_inference, validate_anticipation, validate_n_bootstrap
4949

5050
if TYPE_CHECKING:
5151
from diff_diff.survey import ResolvedSurveyDesign, SurveyDesign
@@ -182,7 +182,8 @@ class ContinuousDiD(_ContinuousDiDAggregationMixin, BaseEstimator):
182182
``P(D=d_L) > 0``) and no never-treated units present. Single-cohort only
183183
(multi-cohort and ``covariates=`` raise ``NotImplementedError``).
184184
anticipation : int, default=0
185-
Number of periods of treatment anticipation.
185+
Number of periods of treatment anticipation. Must be a
186+
non-negative integer; ``bool`` is rejected.
186187
base_period : str, default="varying"
187188
``"varying"`` or ``"universal"``.
188189
alpha : float, default=0.05
@@ -310,12 +311,18 @@ def __init__(
310311
self._validate_constrained_params()
311312

312313
def _validate_constrained_params(self) -> None:
313-
"""Validate control_group, base_period, and estimation_method values."""
314+
"""Validate control_group, base_period, and estimation_method values.
315+
316+
Also validates ``anticipation`` and re-assigns it as a normalized
317+
Python ``int`` — idempotent on an already-normalized value, so a
318+
re-run never changes fitted config.
319+
"""
314320
if self.control_group not in self._VALID_CONTROL_GROUPS:
315321
raise ValueError(
316322
f"Invalid control_group: '{self.control_group}'. "
317323
f"Must be one of {self._VALID_CONTROL_GROUPS}."
318324
)
325+
self.anticipation = validate_anticipation(self.anticipation)
319326
if self.base_period not in self._VALID_BASE_PERIODS:
320327
raise ValueError(
321328
f"Invalid base_period: '{self.base_period}'. "
@@ -442,6 +449,14 @@ def fit(
442449
else:
443450
aggregate = None
444451

452+
# Fit-time re-check: __init__ and set_params validate eagerly, so
453+
# this only catches DIRECT attribute mutation (est.anticipation = ...)
454+
# — an out-of-domain value silently changes the ESTIMAND. The
455+
# assignment also re-normalizes a mutated numpy scalar to int. Placed
456+
# AFTER the deprecation shim so a caller who both mutated and passed
457+
# a deprecated argument still sees the FutureWarning before the raise.
458+
self.anticipation = validate_anticipation(self.anticipation)
459+
445460
# 1. Validate & prepare
446461
_VALID_AGGREGATES = (None, "dose", "eventstudy")
447462
if aggregate not in _VALID_AGGREGATES:

diff_diff/efficient_did.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
compute_omega_star_nocov,
6565
enumerate_valid_triples,
6666
)
67-
from diff_diff.utils import safe_inference, validate_n_bootstrap
67+
from diff_diff.utils import safe_inference, validate_anticipation, validate_n_bootstrap
6868

6969
# Re-export for convenience
7070
__all__ = ["EfficientDiD", "EfficientDiDResults", "EDiDBootstrapResults"]
@@ -290,7 +290,8 @@ class EfficientDiD(EfficientDiDBootstrapMixin, _EfficientAggregationMixin, BaseE
290290
Random seed for reproducibility.
291291
anticipation : int, default 0
292292
Number of anticipation periods (shifts the effective treatment
293-
boundary forward by this amount). When combined with
293+
boundary forward by this amount). Must be a non-negative
294+
integer; ``bool`` is rejected. When combined with
294295
``control_group="last_cohort"``, also trims the pseudo-control
295296
period set at ``t >= last_g - anticipation`` (see REGISTRY.md).
296297
sieve_k_max : int or None
@@ -375,14 +376,20 @@ def __init__(
375376
self._validate_params()
376377

377378
def _validate_params(self) -> None:
378-
"""Validate constrained parameters."""
379+
"""Validate constrained parameters.
380+
381+
Also validates ``anticipation`` and re-assigns it as a normalized
382+
Python ``int`` — idempotent on an already-normalized value, so the
383+
fit-time re-run never changes fitted config.
384+
"""
379385
if self.pt_assumption not in ("all", "post"):
380386
raise ValueError(f"pt_assumption must be 'all' or 'post', got '{self.pt_assumption}'")
381387
if self.control_group not in ("never_treated", "last_cohort"):
382388
raise ValueError(
383389
f"control_group must be 'never_treated' or 'last_cohort', "
384390
f"got '{self.control_group}'"
385391
)
392+
self.anticipation = validate_anticipation(self.anticipation)
386393
valid_weights = ("rademacher", "mammen", "webb")
387394
if self.bootstrap_weights not in valid_weights:
388395
raise ValueError(
@@ -1546,7 +1553,8 @@ def hausman_pretest(
15461553
cluster : str, optional
15471554
Cluster column for cluster-robust covariance.
15481555
anticipation : int
1549-
Anticipation periods.
1556+
Anticipation periods. Must be a non-negative integer; ``bool``
1557+
is rejected.
15501558
control_group : str
15511559
``"never_treated"`` or ``"last_cohort"``.
15521560
alpha : float
@@ -1559,6 +1567,12 @@ def hausman_pretest(
15591567
-------
15601568
HausmanPretestResult
15611569
"""
1570+
# The classmethod uses `anticipation` in its OWN event-time
1571+
# arithmetic (`e < -ant` below), not just forwarding to the two
1572+
# constructed estimators — validate and normalize it here so an
1573+
# unsigned numpy scalar cannot wrap the comparison.
1574+
anticipation = validate_anticipation(anticipation)
1575+
15621576
# Fit under both assumptions (analytical SEs only, no bootstrap)
15631577
common_kwargs = dict(
15641578
cluster=cluster,

0 commit comments

Comments
 (0)