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
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed
- **`n_bootstrap` type guards aligned onto `utils.validate_n_bootstrap`**
for the estimators the M-081 sweep deliberately left out —
`HeterogeneousAdoptionDiD`, `ChaisemartinDHaultfoeuille`, `TROP`,
`SyntheticDiD` (jackknife lane included), plus the two HAD pretest
helpers (`stute_test`, `stute_joint_pretest`): previously-accepted
type-blind values now raise the shared message — `True` (silently ran as
1 replicate on HAD/dCDH), floats like `2.5` (passed the `>= 2` floors),
and bool/negative under SyntheticDiD's jackknife floor exemption. The
estimator-specific floors are unchanged and keep their own messages for
non-negative sub-floor integers (TROP/SDiD `n_bootstrap=1`, HAD `0`);
NEGATIVE values now surface the shared validator's message instead of
each estimator's former wording.
- **`honest_did` inference-df resolution consolidated** onto the shared
`aggregation.resolve_inference_df()` (three duplicated precedence blocks
removed). Same precedence; `HonestDiDResults.df_survey` is now
float-typed (`31.0` where it was `31`), and a fractional `df_inference`
is preserved instead of truncated.

### Fixed
- **Per-row event-study df provenance (M-092 completion)** for the four
remaining holes — `EfficientDiD`, `ImputationDiD`, `ContinuousDiD`, and
Expand Down
2 changes: 0 additions & 2 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ Related tracking surfaces:
| `WooldridgeDiD` DROPS the observations of a cohort with no supported pre-period before `g - anticipation` ([M-123]) rather than identifying it. Excluding the rows is correct given `g-1` normalization -- leaving them in silently loads the cohort's effect onto the time FE -- but dropping a cohort a user supplied is a lossy last resort. **Route (b) is now SETTLED NEGATIVELY and is not the answer:** the paper's no-never-treated last-cohort normalization shipped (W2025 Sec 5.4, per-period comparison support), and it does NOT identify these cohorts -- `wooldridge-2025-review.md:477` is explicit that in the final period the last cohort's ATT is unidentified, and the implementation still excludes any cohort whose reference is `None`. **Route (a) remains open:** an explicit user-supplied reference period per cohort -- W2025 Section 6.1 says any pre-treatment period may serve and the pre-trend `t`-test is invariant to the choice, so a cohort with ANY supported pre-period is a candidate even when `g-1` is missing. If route (a) also fails to identify the cohort, convert this row into a REGISTRY Note recording exclusion as the deliberate final answer. | `diff_diff/wooldridge.py`, `docs/methodology/REGISTRY.md` | #724 | Heavy | Medium |
| `WooldridgeDiD` REFUSES a panel whose units split into disconnected support groups within a cohort, rather than estimating what IS identified. The connectivity guard (REGISTRY *within-cohort support connectivity*) correctly detects that a closed component's cells are collinear with the unit FE — previously QR dropped one silently and the overall ATT averaged an incomplete set (issue #724's failure mode via unit support). **Refusing is the safe answer, not the complete one.** The connected component containing the reference is still fully identified, so the estimable resolution is either (a) estimate the connected component and report the disconnected units as excluded, with the estimand restated (a sub-population of units, so it needs a REGISTRY definition and interacts with the survey-domain row above), or (b) per-component references, if a component with its own pre-period can carry its own normalization — needs a methodology decision, since components then are not comparable on one baseline. Gate with the split-support fixture in `TestWithinCohortSupportConnectivity`. | `diff_diff/wooldridge.py` | #724-codex-R7 | Heavy | Medium |
| `WooldridgeDiD` fully resolves the `SurveyDesign` TWICE on every supported survey fit. The pre-exclusion validation pass (added so invalid metadata cannot hide in rows that cohort exclusion deletes) calls `survey_design.resolve(sample)`, and each fitter then calls `_resolve_survey_for_wooldridge` -> `_resolve_survey_for_fit` on the same frame, repeating weight normalization, strata/PSU/FPC validation and design-array construction. Any fit that REACHES the second resolve has an unchanged sample (survey + unidentified-cohort exclusion raises first), so the first result is reusable: capture the `_resolve_survey_for_fit` 4-tuple early and thread it into the three fitters as an optional `pre_resolved`. **Caveat that makes this non-trivial:** `sample = sample.reset_index(drop=True)` runs BETWEEN the two calls, so the reused object must be verified index-independent (resolution extracts positional numpy arrays, but `_inject_cluster_as_psu` and the metadata recompute need checking), and the early call must stop suppressing warnings or the user loses the weight-normalization notice. Gate with a survey fit asserting one normalization warning and byte-identical SEs. | `diff_diff/wooldridge.py` | #724-codex-R10 | Mid | Low |
| Consolidate the inference-df precedence duplicated across `honest_did.py` (3 copies at ~L655/L836/L1004) onto the shared `resolve_inference_df()` helper added in `diff_diff/aggregation.py`. The copies are correct today; the risk is drift if the survey/replicate precedence changes in one place only. (Adjacent but distinct from PR C's `utils.resolve_tail_df`: that is the FIT-TIME `df_convention` fallback resolver, this is a post-fit results READER.) | `diff_diff/honest_did.py` | #726 | Quick | Low |
| `ContinuousDiD` CGBS-2024 remaining extensions (earlier phases — `covariates=` reg/dr, `treatment_type="discrete"`, single-cohort `control_group="lowest_dose"` with estimand `ATT(d)−ATT(d_L)` — are already supported; see REGISTRY Note #7). Remaining (all deferred `NotImplementedError`, documented): `estimation_method="ipw"` on the dose curve (scalar-adjustment / degenerate); `covariates=` × `survey_design=` (weighted OR + weighted nuisance IF); multi-cohort **heterogeneous-support** discrete aggregation (support-aware: average each dose only over the cohorts that observe it); **multi-cohort `lowest_dose`** (within-cohort `d_L` reference + support-aware cross-cohort aggregation); and **`covariates=` × `lowest_dose`** (conditional-PT-relative-to-`d_L` estimand). Single-cohort / 2-period / shared-support multi-cohort are supported. | `continuous_did.py` | CGBS-2024 | Heavy | Low |
| `WooldridgeDiD` does not apply the W2025 Sec 5.4 `D_{G_max} x X` covariate normalization, and three sibling covariate rank deficiencies are pre-existing. Measured with the period range pinned and only the never-treated units toggled: (1) time-invariant `exovar` is absorbed by the unit FE, 4 of 26 columns, IDENTICALLY with and without never-treated units; (2) `xgvar`'s cell x covariate block, 19 of 41, identical on both panels; (3) `xtvar` under `demean_covariates=False` does exhibit the `sum_g D_g x = x` dependency that the default demeaning removes; (4) the newly-reachable case -- time-VARYING data passed through `exovar`, which its own docstring reserves for time-invariant covariates -- where the paper's `dT_i` rule would give a deterministic `D_{G_max} x X` drop instead of QR's arbitrary pick (coefficients unaffected, `1.35e-14`; `rank_deficient_action="error"` raises). REGISTRY's narrowed Sec 5.4 note cross-references this row. **Trap for whoever takes it:** `xtvar` under the DEFAULT `demean_covariates=True` is FULL RANK -- the raw block carries demeaned values while `D_g x X` carries raw ones -- and forcing the drop there moves `overall_att` 1.11903 -> 1.46269. Pinned as-is by `TestComparisonSupportFiltering::test_cells_derived_groups_did_not_leak_into_the_design`. | `diff_diff/wooldridge.py` | #729-followup | Heavy | Medium |
| `WooldridgeDiD.n_control_units` counts never-treated UNITS on `control_group="never_treated"` regardless of method, but on the nonlinear paths (`logit`/`poisson`) treated units' pre-treatment rows ARE the identifying comparison -- only the OLS path absorbs them into their own cells. So the reported count under-states the comparison pool exactly where the REGISTRY control-pool asymmetry note applies. Widen to `not_yet_treated or (never_treated and method != "ols")`, or document the count as never-treated-units-by-definition. Behavior is PRE-EXISTING; documented for now in the REGISTRY control-pool Note rather than changed, because widening moves a public results field and wants its own ledger row and test matrix. | `diff_diff/wooldridge.py` | #729-followup | Mid | Low |
Expand Down Expand Up @@ -82,7 +81,6 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m
| Replicate Chang (2020) §4's own RCS simulation DGPs (pp. 17-21, "fully specified" per the paper review) as recovery/coverage fixtures for the `DMLDiD(panel=False)` lane — the shipped tests use a library-authored RCS design (documented in the REGISTRY checklist caveat); needs the paper PDF to extract the parameterization | `tests/test_methodology_dml_did.py`, `docs/methodology/papers/chang-2020-review.md` | DML PR-B2 | Mid | Low |
| Optional scheduled end-to-end execution gate for the MMM tutorials (29/30): a cron-only workflow (or extension of `mmm-interop.yml`) that executes both notebooks in isolated exact-pin environments, so a stale/invalid committed posterior cannot stay green indefinitely - today the hybrid posture (deliberate: notebooks execute locally with committed outputs; CI smoke-tests the exporters without sampling; drift tests pin source + committed-output needles) leaves the MCMC claims un-re-executed in CI | `.github/workflows/mmm-interop.yml`, `docs/tutorials/29_mmm_calibration_pymc.ipynb`, `docs/tutorials/30_mmm_calibration_meridian.ipynb` | mmm-interop | Mid | Low |
| Committed `fixest::feols` event-study golden for TWFE `event_study=True` (within + pooled specs, unbalanced + covariate panels, matched CR1 cluster convention, per-period effects + vcov block) - the in-suite gates are shared-core cross-checks (TWFE-within == MPD-absorb, pooled == MPD bit-exact), so a defect common to the shared core would pass; the live-R harness (`benchmarks/R/benchmark_multiperiod.R`, `feols(y ~ treated * time_f \| unit)`) validated the within design in `docs/benchmarks.rst` but is not a committed regression test - follow the `fixest_did_twfe_golden.json` committed-golden pattern (pytest.skip when absent) | `tests/test_fixest_did_twfe_parity.py`, `benchmarks/R/` | 3(a) R2 | Mid | Medium |
| Type-blind `n_bootstrap` acceptance in already-validated estimators - HAD bool (`isinstance(..., int)` passes `True`, runs as 1 replicate), dCDH bool+float (its bare `< 0` check passes both `True` and `2.5`), TROP float (`2.5` passes the `>= 2` floor), SyntheticDiD float under all three variance methods + bool/negative under jackknife (its floor check is skipped there) - align these local checks with the `utils.validate_n_bootstrap` type guard (M-081 kept them out of the sweep: it scoped to previously-UNvalidated estimators only) | `diff_diff/had.py`, `diff_diff/chaisemartin_dhaultfoeuille.py`, `diff_diff/trop.py`, `diff_diff/synthetic_did.py` | 2(d) PR-B | Quick | Low |
| Evaluate adding the `BaseEstimator` param surface (get_params/set_params) to the exported classes that never had it - `PowerAnalysis`, `LinearRegression`, `BusinessReport`, `DiagnosticReport`, `TWFEWeightsResult` (a NEW public surface, deliberately out of the 2(c)-i pure-refactor scope; `LinearRegression` is the one `fit`-bearing class excluded from the contract suite's roster-completeness test). | `diff_diff/linalg.py`, `diff_diff/power.py` | mixin PR | Mid | Low |
| Tighten the mypy suppressions that back the enforced-zero posture: burn down `prep_dgp`'s per-module `[index]` override (needs a None-vs-array restructure that preserves the seeded RNG stream), and evaluate re-enabling the globally disabled codes (`arg-type`, `return-value`, `var-annotated`, `assignment`) one at a time — `assignment` alone hid several real annotation drifts found during the 2026-07 triage. | `pyproject.toml` `[tool.mypy]`, `diff_diff/prep_dgp.py` | lint-CI | Mid | Low |
| Tracking-file contract guard test: reject NEW active deferred-work pointers at `TODO.md` (deferred rows live in `DEFERRED.md`; allowlist for historical/past-tense prose and actionable-row pointers) and assert rows cross-linking a `docs/v4-deprecations.yaml` `M-xxx` id don't restate ledger status. Origin: tracking-split local review R2. | `tests/`, `TODO.md`, `DEFERRED.md` | tracking-split | Quick | Low |
Expand Down
11 changes: 6 additions & 5 deletions diff_diff/chaisemartin_dhaultfoeuille.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
DCDHBootstrapResults,
)
from diff_diff.linalg import solve_ols
from diff_diff.utils import safe_inference
from diff_diff.utils import safe_inference, validate_n_bootstrap

__all__ = [
"ChaisemartinDHaultfoeuille",
Expand Down Expand Up @@ -762,8 +762,8 @@ def __init__(
)
if not 0.0 < alpha < 1.0:
raise ValueError(f"alpha must be in (0, 1), got {alpha}")
if n_bootstrap < 0:
raise ValueError(f"n_bootstrap must be non-negative, got {n_bootstrap}")
# Shared type guard (rejects bool/float, floor stays 0).
validate_n_bootstrap(n_bootstrap)
if by_path is not None:
if isinstance(by_path, bool) or not isinstance(by_path, int):
raise ValueError(
Expand Down Expand Up @@ -836,8 +836,9 @@ def _validate_invariants(self) -> None:
)
if not 0.0 < self.alpha < 1.0:
raise ValueError(f"alpha must be in (0, 1), got {self.alpha}")
if self.n_bootstrap < 0:
raise ValueError(f"n_bootstrap must be non-negative, got {self.n_bootstrap}")
# Shared type guard (kept aligned with __init__ so the two sites
# cannot drift).
validate_n_bootstrap(self.n_bootstrap)
if self.by_path is not None:
if isinstance(self.by_path, bool) or not isinstance(self.by_path, int):
raise ValueError(
Expand Down
9 changes: 5 additions & 4 deletions diff_diff/had.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
SurveyMetadata,
compute_survey_metadata,
)
from diff_diff.utils import safe_inference
from diff_diff.utils import safe_inference, validate_n_bootstrap

__all__ = [
"HeterogeneousAdoptionDiD",
Expand Down Expand Up @@ -3045,9 +3045,10 @@ def _validate_constructor_args(self) -> None:
f"or None."
)
# Phase 4.5 B: n_bootstrap must be a positive int; seed must be
# None or a nonneg int (numpy default_rng contract).
if not isinstance(self.n_bootstrap, (int, np.integer)):
raise ValueError(f"n_bootstrap must be an int; got {type(self.n_bootstrap).__name__}.")
# None or a nonneg int (numpy default_rng contract). The shared
# type guard rejects bool (previously ran as 1 replicate) and
# floats; the >= 1 floor stays HAD-specific.
validate_n_bootstrap(self.n_bootstrap)
if int(self.n_bootstrap) < 1:
raise ValueError(f"n_bootstrap must be >= 1; got {self.n_bootstrap!r}.")
if self.seed is not None:
Expand Down
7 changes: 6 additions & 1 deletion diff_diff/had_pretests.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
SurveyDesign,
make_pweight_design,
)
from diff_diff.utils import _generate_mammen_weights
from diff_diff.utils import _generate_mammen_weights, validate_n_bootstrap

__all__ = [
"QUGTestResults",
Expand Down Expand Up @@ -1676,6 +1676,9 @@ def stute_test(
"""
if not (0.0 < alpha < 1.0):
raise ValueError(f"alpha must satisfy 0 < alpha < 1, got {alpha}.")
# Shared type guard first (a float like 999.5 previously passed the
# floor and reached np.empty(n_bootstrap)).
validate_n_bootstrap(n_bootstrap)
if n_bootstrap < _MIN_N_BOOTSTRAP:
raise ValueError(
f"n_bootstrap must be >= {_MIN_N_BOOTSTRAP} (below this the "
Expand Down Expand Up @@ -2880,6 +2883,8 @@ def stute_joint_pretest(
# Note: the actual `warn + return` happens below after horizon
# labels are validated and collision-checked, so the NaN result
# carries full per-horizon diagnostic keys.
# Shared type guard first (float floors through otherwise).
validate_n_bootstrap(n_bootstrap)
if n_bootstrap < _MIN_N_BOOTSTRAP:
raise ValueError(f"n_bootstrap must be >= {_MIN_N_BOOTSTRAP}; got " f"{n_bootstrap}.")
if not isinstance(alpha, (int, float)) or not (0 < float(alpha) < 1):
Expand Down
Loading
Loading