Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
expression), pointing at `full_model_window=True` as the mroi route.

### Changed
- **Family-wide `anticipation` domain validation ([M-144], landing at 4.0 —
the locked ladder's next release, with no warn-then-error window per the
M-096/M-142 precedent that validation tightenings ship immediately;
retires the TODO "library-wide anticipation domain validation" row).** All
nine anticipation-taking estimators (`CallawaySantAnna`, `SunAbraham`,
`ImputationDiD`, `TwoStageDiD`, `StackedDiD`, `ContinuousDiD`,
`EfficientDiD`, `WooldridgeDiD`, `SpilloverDiD`) now validate
`anticipation` at construction via the shared
`utils.validate_anticipation` (non-negative integer; `bool` rejected;
`set_params` transactional) AND re-check it on the fit path — the uniform
direct-mutation defense, in the assignment form that also normalizes
numpy scalars to built-in `int`. Previously seven of the nine accepted
anything, and an out-of-domain window silently changed the ESTIMAND:
measured, `CallawaySantAnna(anticipation=-1)` moved the overall ATT by
−85% and flipped its sign under `control_group="not_yet_treated"`;
`anticipation=True` fit bit-identically to `1` (a silent one-period
window); `SunAbraham(anticipation=1.5)` returned `att=nan` without
raising. `0` stays legal. The one break of previously-CORRECT code:
whole-valued floats (`anticipation=1.0`, `np.float64(1.0)`) previously
fit bit-identically to their integer value on
CS/SunAbraham/ImputationDiD/TwoStageDiD/EfficientDiD/WooldridgeDiD and
now raise — use the int (StackedDiD already crashed on floats via an
incidental `range()` TypeError, now a clear constructor `ValueError`;
ContinuousDiD's float behavior was fixture-dependent). Also visible:
accepted numpy integers are retyped — the public `anticipation`
attribute and `get_params()["anticipation"]` are now always built-in
`int`, not a numpy scalar; `WooldridgeDiD`'s message text changed to the
shared wording, its `None`/str raw `TypeError` became `ValueError`, and
its constructor error ordering moved (bad
`bootstrap_weights`/`vcov_type`/`df_convention` now report before a bad
`anticipation`); `SpilloverDiD`'s raise moved from fit to construction
(the fit-time re-check is retained, ordered before the ref-period
arithmetic), and its negative-int message dropped the `(type ...)`
suffix (shared text). `EfficientDiD.hausman_pretest` normalizes its own
`anticipation` argument (an unsigned numpy scalar previously wrapped its
event-time arithmetic and silently degraded the pretest to an all-NaN
inconclusive result). The deprecated `StaggeredTripleDifference` stays
construction-permissive by design (fit-validated via the shared engine).
Both LLM guides note the domain; policy suite:
`tests/test_anticipation_policy.py`.
- **DiagnosticReport's event-study-gated checks now consume the post-fit
`results.aggregate('event_study')` surface** (the 3.9 M-020 family;
retires the TODO "diagnostic_report ES-gated checks" row): on a modern
Expand Down
1 change: 0 additions & 1 deletion TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ Related tracking surfaces:
| 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 |
| 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 |
| 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 |
| 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 |
| `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 |
| 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 |
| 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 |
Expand Down
13 changes: 9 additions & 4 deletions diff_diff/_staggered_triple_diff_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,13 @@ class _StaggeredTripleDiffEngineMixin:
supply the constructor attributes and the CS aggregation/bootstrap mixins
this core calls. The annotations below exist because mypy type-checks this
class independently of its hosts (`attr-defined` is not disabled) - they are
declarations, never assignments.
declarations, never assignments — with ONE exception: `anticipation` is
re-assigned by the core's fit-time re-validation (the mutation-defense
re-check normalizes it to a Python int; see `_fit_staggered_core`).
"""

# Constructor attributes read from the host class.
# Constructor attributes read from the host class. (`anticipation` is
# additionally RE-ASSIGNED at fit — the validate-and-normalize re-check.)
estimation_method: str
control_group: str
alpha: float
Expand Down Expand Up @@ -178,8 +181,10 @@ def _fit_staggered_core(
# the deprecated StaggeredTripleDifference (whose 3.x API SHAPE is
# frozen through removal - that freeze was never a licence to emit
# silently-biased numbers) and direct attribute mutation on either
# class, which bypasses __init__ and set_params alike.
validate_anticipation(self.anticipation)
# class, which bypasses __init__ and set_params alike. The
# assignment form also normalizes a numpy scalar to a Python int
# before any `g - 1 - anticipation` arithmetic can overflow.
self.anticipation = validate_anticipation(self.anticipation)
from diff_diff.survey import (
_resolve_survey_for_fit,
_validate_unit_constant_survey,
Expand Down
21 changes: 18 additions & 3 deletions diff_diff/continuous_did.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
build_unit_first_row_index,
compute_survey_vcov,
)
from diff_diff.utils import safe_inference, validate_n_bootstrap
from diff_diff.utils import safe_inference, validate_anticipation, validate_n_bootstrap

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

def _validate_constrained_params(self) -> None:
"""Validate control_group, base_period, and estimation_method values."""
"""Validate control_group, base_period, and estimation_method values.

Also validates ``anticipation`` and re-assigns it as a normalized
Python ``int`` — idempotent on an already-normalized value, so a
re-run never changes fitted config.
"""
if self.control_group not in self._VALID_CONTROL_GROUPS:
raise ValueError(
f"Invalid control_group: '{self.control_group}'. "
f"Must be one of {self._VALID_CONTROL_GROUPS}."
)
self.anticipation = validate_anticipation(self.anticipation)
if self.base_period not in self._VALID_BASE_PERIODS:
raise ValueError(
f"Invalid base_period: '{self.base_period}'. "
Expand Down Expand Up @@ -442,6 +449,14 @@ def fit(
else:
aggregate = None

# Fit-time re-check: __init__ and set_params validate eagerly, so
# this only catches DIRECT attribute mutation (est.anticipation = ...)
# — an out-of-domain value silently changes the ESTIMAND. The
# assignment also re-normalizes a mutated numpy scalar to int. Placed
# AFTER the deprecation shim so a caller who both mutated and passed
# a deprecated argument still sees the FutureWarning before the raise.
self.anticipation = validate_anticipation(self.anticipation)

# 1. Validate & prepare
_VALID_AGGREGATES = (None, "dose", "eventstudy")
if aggregate not in _VALID_AGGREGATES:
Expand Down
22 changes: 18 additions & 4 deletions diff_diff/efficient_did.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
compute_omega_star_nocov,
enumerate_valid_triples,
)
from diff_diff.utils import safe_inference, validate_n_bootstrap
from diff_diff.utils import safe_inference, validate_anticipation, validate_n_bootstrap

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

def _validate_params(self) -> None:
"""Validate constrained parameters."""
"""Validate constrained parameters.

Also validates ``anticipation`` and re-assigns it as a normalized
Python ``int`` — idempotent on an already-normalized value, so the
fit-time re-run never changes fitted config.
"""
if self.pt_assumption not in ("all", "post"):
raise ValueError(f"pt_assumption must be 'all' or 'post', got '{self.pt_assumption}'")
if self.control_group not in ("never_treated", "last_cohort"):
raise ValueError(
f"control_group must be 'never_treated' or 'last_cohort', "
f"got '{self.control_group}'"
)
self.anticipation = validate_anticipation(self.anticipation)
valid_weights = ("rademacher", "mammen", "webb")
if self.bootstrap_weights not in valid_weights:
raise ValueError(
Expand Down Expand Up @@ -1544,7 +1551,8 @@ def hausman_pretest(
cluster : str, optional
Cluster column for cluster-robust covariance.
anticipation : int
Anticipation periods.
Anticipation periods. Must be a non-negative integer; ``bool``
is rejected.
control_group : str
``"never_treated"`` or ``"last_cohort"``.
alpha : float
Expand All @@ -1557,6 +1565,12 @@ def hausman_pretest(
-------
HausmanPretestResult
"""
# The classmethod uses `anticipation` in its OWN event-time
# arithmetic (`e < -ant` below), not just forwarding to the two
# constructed estimators — validate and normalize it here so an
# unsigned numpy scalar cannot wrap the comparison.
anticipation = validate_anticipation(anticipation)

# Fit under both assumptions (analytical SEs only, no bootstrap)
common_kwargs = dict(
cluster=cluster,
Expand Down
Loading
Loading