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
4 changes: 2 additions & 2 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ Related tracking surfaces:
| Opt-in singleton-group pruning for TwoWayFixedEffects (static + event-study mode; reghdfe parity): singleton units/periods are currently RETAINED class-wide - the within-demeaned row is zero so points are unchanged, but N/G/residual-df count it and CR1/finite-sample SEs shift (~0.41019 -> 0.40962 measured; REGISTRY "Deviation from R" Note, R5 review) - reghdfe iteratively drops singletons by default while fixest retains them (diff-diff matches fixest); an opt-in knob needs iterative unit+period pruning with consistent cluster/survey/replicate/Conley array subsetting and a default-flip decision protocol (moves published SEs) | `diff_diff/twfe.py`, `diff_diff/estimators.py`, `diff_diff/utils.py` | 3(a) R5 | Mid | Low |
| Cohort-timing validation input for the simultaneous-adoption event-study family (TWFE `event_study=True` + MultiPeriodDiD through 3.9): an optional `first_treat=`/`cohort=` column so simultaneous adoption becomes checkable under the contract-valid time-invariant `D_i` indicator - today the staggered-adoption advisory derives timing from within-unit 0->1 transitions, so it can only fire on off-contract time-varying `D_it` input, and with valid `D_i` adoption timing is not observable in the inputs at all (REGISTRY "staggered-adoption detection limit" Notes, both sections); design questions: validate-only vs steering error, and interplay with the M-011 removal | `diff_diff/twfe.py`, `diff_diff/estimators.py` | 3(a) R2 | Mid | Medium |
| CS parity with DMLDiD's replicate conventions: per-cell replicate SEs for `CallawaySantAnna` (CS leaves per-cell SEs on the weighted sqrt-sum under replicate designs; DMLDiD now computes them via `compute_replicate_if_variance`) AND flip the replace-style effective-df relays to min-cap (`min(df_survey, n_valid - 1)`) at all five sites: CS overall (`staggered.py` overall relay), the shared event-study/group aggregation sites (`staggered_aggregation.py` `df_survey_val = min(non_none_dfs)` x2), and the staggered-DDD engine twins (`_staggered_triple_diff_engine.py` overall + event-study) — the replace convention can RAISE df above the QR-rank design df (anti-conservative); latent, fires only when `n_valid < R` (all-zero/non-finite replicate column). DMLDiD's min-cap + the pinned inherited ES/group behavior are the templates (REGISTRY DMLDiD replicate Note) | `diff_diff/staggered.py`, `diff_diff/staggered_aggregation.py`, `diff_diff/_staggered_triple_diff_engine.py` | DML replicate PR | Mid | Low |
| ImputationDiD/TwoStageDiD `aggregate()` recompute levels on bootstrapped fits fail closed ('simple' relays since the M-027 per-level convergence; M-021/M-022); ImputationDiD's per-target psi machinery makes seeded replay tractable (the panel-backed kit retains everything the psi precompute reads), TwoStageDiD's per-level GMM scores are function-locals and would need retention | `diff_diff/imputation_results.py`, `diff_diff/two_stage_results.py`, `diff_diff/aggregation.py` | 2(b) PR-3b | Mid | Low |
| ContinuousDiD `aggregate('event_study')` on bootstrapped fits fails closed (M-025); a seeded post-fit bootstrap-ES replay is tractable - the multiplier draws are seeded (`np.random.default_rng(self.seed)`) - but needs the FULL per-cell `_bootstrap_info` (bread/ee_treated/Psi_eval/dPsi_*/beta_pred) the pruned kit deliberately drops, so shipping it means a kit-payload change with its own memory contract | `diff_diff/continuous_did_aggregation.py`, `diff_diff/continuous_did_results.py` | 2(b) PR-3c | Mid | Low |
| `balance_e` replay on bootstrapped ImputationDiD/TwoStageDiD fits: the unrestricted ES/group surface is relayed since 4.0 PR 3c (computed at fit, `balance_e=None`; M-021/M-022), but `aggregate('event_study', balance_e=k)` still raises there - a balanced-window replay needs retained per-target psi (ImputationDiD: the panel-backed kit retains everything the psi precompute reads, so a seeded replay is tractable) / retained per-level GMM scores (TwoStageDiD: function-locals today) | `diff_diff/imputation_results.py`, `diff_diff/two_stage_results.py`, `diff_diff/aggregation.py` | 2(b) PR-3b | Mid | Low |
| ContinuousDiD post-fit bootstrap-ES REPLAY: since 4.0 PR 3c a bootstrapped fit relays the event-study surface computed at fit (M-025), so the gap is a genuine post-fit replay (fresh draws, no fit-side cost, a route for pre-4.0 pickles); a seeded replay is tractable - the multiplier draws are seeded (`np.random.default_rng(self.seed)`) - but needs the FULL per-cell `_bootstrap_info` (bread/ee_treated/Psi_eval/dPsi_*/beta_pred) the pruned kit deliberately drops, so shipping it means a kit-payload change with its own memory contract | `diff_diff/continuous_did_aggregation.py`, `diff_diff/continuous_did_results.py` | 2(b) PR-3c | Mid | Low |
| practitioner `step_name="heterogeneity"` producer-side collisions: three OTHER estimators' advice steps reuse the key with non-heterogeneity labels (`:975` ContinuousDiD dose-response, `:1022` Triple placebo-group, `:1413` LPDiD WAS arrays), so DiagnosticReport's heterogeneity completion silently drops that unrelated advice from `next_steps` via `_filter_steps` - the same latent collision fixed for StackedDiD in M-024 (renamed to `sub_experiment_balance`). Renaming these changes those estimators' report output; audit + rename with per-estimator pins. | `diff_diff/practitioner.py` | 2(b) PR-2 review R9 | Quick | Low |
| PreTrendsPower `violation='linear'` on CS `base_period='varying'` input targets the wrong alternative: `δ_pre = M · \|t\|` assumes level coefficients against a common reference, but varying-base pre-treatment effects are consecutive-period comparisons (constant increments under a linear trend). Both CS-sourced routes now WARN (REGISTRY PreTrendsPower Note), and universal-base GAPPED grids fail closed via the `reference_event_times` common-reference guard; what remains is the varying-base resolution - either transforming the violation vector through each coefficient's actual base mapping (needs per-horizon base provenance) or requiring `base_period='universal'` for the linear benchmark - a per-estimator methodology decision with a hand-calculated linear-violation gate | `diff_diff/pretrends.py` | 2(b) PR-1 R5 | Mid | Medium |
| `run_placebo_test`/`run_all_placebo_tests` expose ONE overloaded `time` param (calendar for timing/group tests, 0/1 post dummy forwarded into the M-137/M-138 callees - works only because a two-period 0/1 calendar column is both); redesign the wrapper signatures (dual `time=`/`post=` params or a split) so each callee gets an honestly-named argument | `diff_diff/diagnostics.py` | 2(c)-ii PR-B | Mid | Medium |
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/python/benchmark_imputation.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def main():
first_treat="first_treat",
)
# Post-fit event study stays inside the timed region (the 3.x
# fit(aggregate="event_study") route, deprecated since 3.9 and removed at the 4.0 cut).
# fit(aggregate="event_study") route, removed at 4.0).
es = results.aggregate("event_study")

estimation_time = estimation_timer.elapsed
Expand Down
85 changes: 85 additions & 0 deletions changelog.d/20261021-v4-pr3c-aggregate-relay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
### Removed
- **The last fit-time `aggregate=` / `balance_e=` keywords are gone (ledger rows flipped to
`removed`).** `fit(aggregate=)` is removed from `ImputationDiD` (`M-021`), `TwoStageDiD`
(`M-022`) and `ContinuousDiD` (`M-025`), `fit(balance_e=)` from `ImputationDiD` (`M-118`)
and `TwoStageDiD` (`M-119`). With PR 3b's removals on CallawaySantAnna / EfficientDiD /
StackedDiD / dCDH / HAD this completes the aggregate-postfit group: every adopter aggregates
through post-fit `results.aggregate(type, balance_e=)` only, and `TripleDifference`'s
staggered-mode keywords are the one surviving fit-time `aggregate=` (`M-140`/`M-141`,
canonical until the DDD container unification). With the keywords go ContinuousDiD's
no-underscore `"eventstudy"` spelling and its fit-time value validation, the dead analytic
fit-time event-study inference arm in `continuous_did.py`, and ImputationDiD's unreachable
pretrends+replicate fit-time gate.

### Breaking Changes
- Passing `aggregate=` or `balance_e=` to `ImputationDiD.fit`, `TwoStageDiD.fit` or
`ContinuousDiD.fit` now raises Python's native `TypeError: ... unexpected keyword argument`.
- **Positional slots after the removed parameters shift left.** `ImputationDiD` and
`TwoStageDiD`: `survey_design` 9 -> 7; `ContinuousDiD`: `survey_design`, `covariates`
8/9 -> 7/8. Pass these by keyword.
- `aggregate('event_study', balance_e=k)` on a BOOTSTRAPPED `ImputationDiD` / `TwoStageDiD`
fit raises `NotImplementedError` (the relayed surface was computed unrestricted; re-fit with
`n_bootstrap=0` for a balanced window). Analytic fits recompute with `balance_e` as before.
- A bootstrapped `ImputationDiD` / `TwoStageDiD` / `ContinuousDiD` fit WITH bootstrap inference
but WITHOUT a stored surface - a 3.9-3.13 bootstrapped plain-fit pickle, or a fit whose
fail-soft materialization dropped it - raises `ValueError` from `aggregate('event_study')` /
`aggregate('group')` (previously `NotImplementedError` from the bootstrap gate); the message
names the refit. A 3.9-3.13 bootstrapped pickle fitted with `balance_e=k` stores a restricted
surface and the relay returns it as stored.

### Changed
- **Bootstrapped `ImputationDiD` / `TwoStageDiD` / `ContinuousDiD` fits relay their event-study
(and, on Imputation/TwoStage, group) surface.** The deprecated keyword was the only route to
bootstrap event-study inference there (no per-draw state is retained for a replay), so a fit
with `n_bootstrap > 0` now ALWAYS computes those surfaces at fit - exactly what
`fit(aggregate="all")` computed, `balance_e=None` - stores them in the legacy
`event_study_effects` / `group_effects` fields, and `aggregate('event_study')` /
`aggregate('group')` RELAYS them (kit-snapshot alpha / anticipation / survey metadata; NaN
`df` column; `vcov` `None`) instead of raising `NotImplementedError`. Analytic fits are
unchanged (legacy fields `None`, kit recompute on demand). Storage rule: the legacy fields
hold a surface IFF the fit carries bootstrap inference. On Imputation/TwoStage a bootstrapped
fit that ends analytical (zero valid draws, total bootstrap failure) clears them and
`aggregate()` recomputes analytically. ContinuousDiD's bootstrapped kit is scalars-only, so
no analytical fallback exists there: a bootstrapped fit with no post-treatment cells never
runs its bootstrap (overall ATT already NaN with a `UserWarning`), stores no surface, and
`aggregate('event_study')` raises the absent-surface `ValueError`.
- Materialization is FAIL-SOFT with a one-shot overall-only retry: an exception while
building the surfaces, or a bootstrap attempt that raises / returns `None` with surfaces
present, leaves them `None`, emits one `UserWarning` ("... could not be materialized ...")
and re-runs the bootstrap overall-only (a seeded fit reproduces the first attempt's weight
draws), so a plain bootstrapped fit's headline quintet and its success/abort outcome are
unchanged by the removal. ContinuousDiD bootstrap failures still propagate as before.
- Consequences on bootstrapped plain fits: the aggregators' own `UserWarning`s (ImputationDiD
Prop-5 unidentified horizons, empty horizons, LOO single-unit auxiliaries; TwoStageDiD
Stage-1/Stage-2 singular or rank-deficient fallbacks) and the per-target bootstrap-stats
`RuntimeWarning`s now fire where a plain fit never emitted them; `summary()` prints the
event-study/group tables and `to_dataframe(level="event_study"|"group")` returns frames
instead of raising; `DiagnosticReport` / `BusinessReport` run heterogeneity (Imputation /
TwoStage on the relayed group dict, ContinuousDiD on the event study) and parallel trends
(Imputation / TwoStage) on the relayed surface instead of skipping with
`NotImplementedError`; `plot_event_study(results)` plots them natively. Measured cost (199
draws, 300 units x 12 periods): Imputation 0.01s -> 0.08s, TwoStage 0.01s -> 0.04s,
Continuous unchanged at 0.05s; all three default to `n_bootstrap=0`.
- `ImputationDiD`'s inert-`df_convention` warning predicate is now exact: the per-lead inference
is reachable iff `pretrends=True` and the design is not replicate-weighted (`M-127`); the
unreachable pretrends+replicate fit-time gate is deleted (the combination is still rejected
post-fit by `aggregate('event_study')`).
- Runtime messages (`plot_event_study`, the results-class "not stored" hints,
`_reporting_helpers`, `DiagnosticReport` docstrings), `practitioner_next_steps` advice for
bootstrapped ImputationDiD / TwoStageDiD / ContinuousDiD fits, the `llms*.txt` guides, the
ContinuousDiD API page and tutorial, the migration guide and the performance docs describe
the relay instead of steering users to the removed fit-time route.

### Internal
- `tests/test_aggregate_contract.py`: the remaining `Test*Shim` classes, fit-time fixtures and
fail-closed bootstrap pins are retired for `Test*Removal` pins (TypeError, signature,
positional-slot binding) and relay pins - the relayed surfaces equal the stored legacy dicts
row-for-row and match the per-backend bootstrap oracles - plus failure-injection pins for the
fail-soft materialization and the storage rule. The estimator suites, `DiagnosticReport`,
`event_study_surface` (bootstrapped Imputation / TwoStage / Continuous enrolled as genuine
native producers) and practitioner tests take their relay pins; the module-level
FutureWarning filters for the fit-time route are gone. Naming-guard allowlist entries for the
guides' post-fit `aggregate` vocabulary are finalized. Ledger row `M-093` gains the fit-side
table of which producers populate the legacy fields; `M-127` records the exact predicate.
`docs/v4-design.md` section 6 carries the relay amendment and the HYBRIDS paragraph; section 9
marks sub-item 3c shipped.
12 changes: 6 additions & 6 deletions diff_diff/_deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
caller-declared default when neither is passed.
- Dropped parameter (no successor name): the parameter keeps its position
with a sentinel default; the owner calls ``warn_deprecated_kwarg`` when a
value was actually supplied. (The 3.9 fit-time ``aggregate=`` shims on
CallawaySantAnna, EfficientDiD, StackedDiD, ChaisemartinDHaultfoeuille and
HeterogeneousAdoptionDiD were this shape; they were deleted at the 4.0 cut
(PR 3b) and ImputationDiD / TwoStageDiD / ContinuousDiD follow in PR 3c.
``warn_deprecated_kwarg`` has no library caller any more (dCDH was the
last) and is retained per section 8 rule 11 for the 5.0 window.)
value was actually supplied. (The 3.9 fit-time ``aggregate=`` shims were
this shape; all of them were deleted at the 4.0 cut - CallawaySantAnna,
EfficientDiD, StackedDiD, ChaisemartinDHaultfoeuille and
HeterogeneousAdoptionDiD in PR 3b, ImputationDiD / TwoStageDiD /
ContinuousDiD in PR 3c. ``warn_deprecated_kwarg`` has no library caller
any more and is retained per section 8 rule 11 for the 5.0 window.)
- Renamed results field: the dataclass field takes the new name and
``deprecated_field_property`` builds the read-only warning alias under the
old name (a plain class attribute, so it is a descriptor but NOT a
Expand Down
4 changes: 2 additions & 2 deletions diff_diff/_reporting_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def describe_target_parameter(results: Any) -> Dict[str, Any]:

- ``ImputationDiD`` / ``TwoStageDiD``: horizon / group tables come
from post-fit ``results.aggregate('event_study'/'group')`` since
3.9 (the fit-time ``aggregate`` kwarg is deprecated) and never
3.9 (the fit-time ``aggregate`` kwarg was removed at 4.0) and never
change ``overall_att``. The headline is always the sample-mean
overall ATT (per BJS 2024 Step 3 with ``w_it = 1/N_1``);
disambiguate via the event-study or group aggregate if you need
Expand Down Expand Up @@ -250,7 +250,7 @@ def describe_target_parameter(results: Any) -> Dict[str, Any]:
"imputed from a unit+time fixed-effects model fitted on "
"untreated observations only (BJS 2024 Step 3). Post-fit "
"``results.aggregate('event_study'/'group')`` (3.9; the "
"fit-time ``aggregate`` kwarg is deprecated) yields the "
"fit-time ``aggregate`` kwarg was removed at 4.0) yields the "
"horizon / group tables and does NOT change ``overall_att`` "
"— for the horizon or group estimand, consult the "
"event-study or group aggregate directly."
Expand Down
22 changes: 20 additions & 2 deletions diff_diff/aggregation.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Post-fit aggregation surface (4.0 program Phase 2, spec section 6).

``results.aggregate(type=...)`` replaces the fit-time ``fit(aggregate=)``
argument (ledger rows M-020..M-027): estimate once, aggregate as a post-fit
``results.aggregate(type=...)`` is the one aggregation surface - the fit-time
``aggregate=`` keyword was removed at 4.0 (ledger rows M-020..M-027): estimate once, aggregate as a post-fit
step - the ecosystem's strongest norm (``did::aggte``, ``etwfe::emfx``, Stata
``estat aggregation``).

Expand Down Expand Up @@ -72,6 +72,24 @@
)


def absent_bootstrap_surface_message(cls_name: str, level: str) -> str:
"""Shared ValueError text for a bootstrapped fit storing no relay surface.

Bootstrapped ImputationDiD / TwoStageDiD / ContinuousDiD fits materialize
their event-study (and group) surfaces at fit and ``aggregate()`` RELAYS
them (4.0 PR 3c). A fit WITH bootstrap inference but WITHOUT the stored
surface is either a pre-4.0 pickle (a bootstrapped plain fit built none)
or a fit whose fail-soft materialization dropped it; both raise this.
"""
return (
f"This bootstrapped {cls_name} carries no stored {level} surface: either it "
"was fitted before 4.0 (a bootstrapped plain fit built none) or the surface "
"could not be materialized at fit (see the fit-time warning). Re-fit with "
"diff-diff >= 4.0 for the relayed surface, or with n_bootstrap=0 for the "
"analytical post-fit recompute."
)


def resolve_inference_df(results: Any) -> Optional[float]:
"""Return the degrees of freedom that governed ``results``' overall statistic.

Expand Down
Loading
Loading