feat(v4): post-fit results.aggregate() contract + CallawaySantAnna (Phase 2b PR 1)#728
Merged
Conversation
Groundwork for Phase 2b PR 1 (spec section 6, ledger rows M-020..M-027).
New diff_diff/aggregation.py:
- AggregationResult(BaseResults): the tabular post-fit aggregation container
with a pinned 12-column schema and summary/to_dict/to_dataframe. Three
choices, each grounded in what CS actually produces rather than assumed:
"target" is PER-ROW so one container can carry two aligned estimands over
the same labels; "n_kind" uses EventStudyResults' existing vocabulary
("groups"/"cells"/"obs"/"clusters") rather than inventing units/obs; and
"weight" is nullable, because CS's group aggregation weights (g,t) cells
equally WITHIN each cohort and has no cross-cohort mass to normalize -
populating it would be a fabricated number.
- AggregationKit: the compact fit-time payload (bookkeeping + influence +
alpha/anticipation/cband), excluding the data matrices.
- BootstrapReplaySpec: a value-bound replay description. Retaining the
estimator's ReplayableWeightStream does not work - it holds a function-local
closure (unpicklable) whose body reads self.n_bootstrap and
self.bootstrap_weights LAZILY, so a post-fit set_params silently truncates
the replay. Recording the generator state plus the parameters BY VALUE and
rebuilding through a module-level factory replays bit-identically, pickles
in ~255 bytes, and is immune to later mutation.
- AggregationMixin: shared validation and dispatch, applied PER RESULTS CLASS.
Never on BaseResults: resolve_locator uses inspect.getattr_static, which
walks the MRO, so a base-class aggregate() would make every still-planned
ledger row's "new" locator resolve and fail test_row_matches_reality.
Pure CS event-study aggregator:
_aggregate_event_study returned only its effects dict and stashed four more
values on self (_event_study_overall / _df_used / _vcov / _vcov_index). It now
returns EventStudyAggregation carrying all five, so it can run post-fit from a
retained kit without mutating its host. StaggeredTripleDifference inherits the
same mixin and is the ONLY reader of _event_study_overall (its Eq. 4.14
overall_att_es), so it is updated in the same diff. fit()'s stale-state reset
is gone: the cross-fit leak it guarded against is now impossible by
construction.
Numerically inert. A 10-scenario golden captured BEFORE the refactor (all four
aggregate values, a balance_e sweep, repeated cross-sections, a bootstrapped
fit and anticipation=1) is reproduced at atol=rtol=1e-14. 980 tests pass
across the staggered, SDDD, event-study-surface, pretrends, honest-did,
diagnostic-report, R-parity and ledger suites.
The post-fit aggregation surface itself (spec section 6, rows M-020 / M-117).
res = CallawaySantAnna().fit(df, ...) # no aggregate= argument
res.aggregate("event_study") # -> EventStudyResults
res.aggregate("group") # -> AggregationResult
res.aggregate("simple") # -> AggregationResult
This gives EventStudyResults its first public producer: the container shipped
in 3.9 (row M-092) has been exported with only package-internal builders.
Retention. fit() now attaches an AggregationKit, built there because neither
`precomputed` nor `influence_func_info` survives the call - both are locals, so
the kit cannot be reconstructed afterwards. It copies only the twelve
bookkeeping keys the aggregation machinery actually reads, verified by
enumerating every access in staggered_aggregation.py; the data matrices
(outcome_matrix, covariate_matrix, obs_outcome, obs_covariates) are excluded,
so a results object never holds the source panel. Asserted: no retained
attribute is a DataFrame.
Reaching the aggregators without an estimator reference. They are mixin
methods, but read exactly two attributes from their host - alpha and
anticipation - so _KitAggregator supplies those and nothing else. That avoids
an _estimator_ref, which would drag the whole fitted estimator, and its frame,
onto every result.
Frame-free aggregation. Three `df` dereferences in the influence-function path
now prefer the precomputed bookkeeping, which the adjacent branches already
did: `unit_cohorts` is the per-unit cohort array, so counting its matches is
identical to the frame lookup it replaces. The unconditional
`assert df is not None` becomes a fail-closed check that accepts EITHER the
frame or the bookkeeping. The frame path remains for direct internal callers.
Immutability. aggregate() returns a new object and never touches its parent.
The event-study builder reads its surface off a results object, so a throwaway
carrier (dataclasses.replace) holds the freshly computed values rather than
populating self. The `_agg_cache` memo write lands on a shallow copy of the
kit's bookkeeping - sharing every array, duplicating no data - mirroring what
StaggeredTripleDifference already does when it aggregates through a modified
copy.
Deprecation shim. fit(aggregate=) and fit(balance_e=) warn (FutureWarning,
removed at 4.0) via a sentinel default, so the warning fires only when the
caller actually supplies the argument and never on a plain fit(). Routing is
untouched: the deprecated path still returns the fully populated legacy surface
that honest_did, pretrends and build_event_study_surface read.
Fail-closed rather than silently wrong: aggregate("calendar") and
aggregate("all") raise naming the supported set; a non-None `weights` raises
(CS exposes no weighting selector); balance_e with "simple"/"group" raises
rather than being ignored, since the shipped code threads it only through
event-study aggregation; and aggregate() on a BOOTSTRAPPED fit raises, because
its percentile inference cannot be reproduced from analytical state. Bootstrap
replay is deliberately not wired here - BootstrapReplaySpec is the verified
mechanism for the follow-up.
Verified: post-fit aggregate() reproduces fit-time numbers IDENTICALLY at
atol=rtol=1e-14 for all three types; the parent is unchanged after five
mixed-order calls and repeated calls agree; the 10-scenario pre-refactor golden
still matches; 499 tests pass across the staggered, SDDD, event-study-surface
and R-parity suites.
Completes Phase 2b PR 1.
Ledger (99 -> 101 rows; floor, id ranges and the snapshot assertion move
together):
- M-020 flips planned -> shimmed and gains a test_ref (it carried none). Its
notes now record the CS SUPPORTED SUBSET: the closed vocabulary is
library-wide, but CallawaySantAnna implements simple|event_study|group and
has no calendar aggregator, so aggregate('calendar') raises naming what is
supported. The DEFERRED "Calendar-time aggregation" row stands.
- M-117 (new): balance_e moves from fit() onto aggregate(). It existed only as
the prose "balance_e moves to aggregate() in the same PR" inside M-020's
notes, which nothing could assert - the same un-rowed-obligation class the
gating-completeness amendment closed. The other three balance_e sites get
rows in the PRs that migrate them.
- M-122 (new): AggregationResult, introduced_in 3.9 with deprecated_in null so
the early-flip guard does not fire against the PR that ships it.
- Ids M-116 and M-118..M-121 are left UNUSED and the gap is documented: they
were drafted for later 2b PRs and are reserved, not deleted, since ids are
never reused.
tests/test_aggregate_contract.py (33 tests) is the test_ref for all three rows:
numerical inertness against fit-time aggregation for every type plus a
balance_e sweep; immutability across mixed-order and repeated calls; the kit
holding no DataFrame and surviving a pickle round-trip; the whole fail-closed
surface (calendar / all / unknown type / weights / balance_e-where-inert /
bootstrapped fit); the pinned schema, nullable weight and declared n_kind; the
shim warning only when the argument is passed; and a StaggeredTripleDifference
regression for the Eq. 4.14 overall that the purity refactor moved off `self`.
Spec correction. Section 6 claimed "CallawaySantAnna already stores them" of
influence functions. It does not - the field was declared and never assigned -
so the text is corrected in place: retention is work in EVERY migrating PR, the
kit must be built during fit() because nothing it needs survives the call, and
the dominant cost is the per-(g,t) influence dict at ~O(n_units x n_gt), not
the O(n_units) bookkeeping.
Guidance migration (section 8 rule 11 - a rename's scope includes every
reader), CS-scoped: the _ABSENT_SURFACE_HINTS entry, honest_did's and
pretrends' CS-gated error messages, and README's step-7 line all told users to
re-run fit with aggregate=; they now point at the post-fit call and say no
refit is required. AggregationResult is added to docs/api/results.rst and the
canonical autosummary in docs/api/index.rst (the CI-enforced IA invariant).
1009 tests pass across the contract, ledger, docs-IA, doc-deps, staggered,
SDDD, honest-did, pretrends and event-study-surface suites.
…stale guidance
Local review round 1. Every finding below was reproduced before being fixed.
df is per-row PROVENANCE - the degrees of freedom that actually produced that
row's stored p-value and interval - so reading whichever df field happened to
be populated was wrong twice:
- "simple" read df_inference, which the field's OWN docstring documents as
staying None on explicit survey_design= fits (the canonical carrier there is
survey_metadata.df_survey). On an explicit PSU design the container reported
df=NaN - implying normal or undefined inference - while the CI it carried was
built on a finite t(16) reference.
- "group" read event_study_df: a DIFFERENT aggregation's denominator, and None
after a plain fit.
Both now resolve from the carrier that governed the statistic.
_aggregate_by_group records the df its own safe_inference_batch used (the
conservative min under dropped replicates, recorded iff it governs a
t-reference), and a shared resolve_inference_df() encodes the
survey_metadata-then-df_inference precedence rather than adding a fourth
independent copy of it - honest_did.py has three, tracked for consolidation.
Container normalization:
- df was normalized with asarray, so NaN-ing the non-estimable rows wrote
THROUGH to the caller's array (verified: [10., 20.] became [10., nan],
shares_memory True) and raised on a read-only buffer. Now np.array, matching
EventStudyResults' normalization of the same field.
- The 1-d check on label ran AFTER shape[0], so a 0-d label raised IndexError
instead of the documented ValueError.
- n_kind emitted "units"/"cells", neither in the vocabulary the docs claimed it
shared with EventStudyResults. The vocabulary is now a single exported
N_KIND_VOCABULARY covering both containers, validated on construction, with
"cells" and "units" documented as members rather than silent extras.
Stale guidance. The messages this branch rewrote pointed at a path that does
not work: HonestDiD, PreTrendsPower AND plot_event_study all dispatch on
CallawaySantAnnaResults and reject the EventStudyResults that aggregate()
returns, so "call results.aggregate('event_study')" ended in a TypeError. They
now say plainly that these consumers read the fit-time surface and do not yet
accept the post-fit container. Making them accept it needs an adapter plus
base_period/anticipation provenance the unified container does not carry - a
scope call, not a message fix. Two CS sites the first sweep missed
(to_dataframe's event_study and group branches) are migrated; the estimator
docstring no longer demonstrates the deprecated kwarg or a plot call that
would raise.
Tests (47 in the contract suite, up from 33): repeated-cross-section and
anticipation=1 round-trips at 1e-14 - designs the balanced-panel fixture never
reached, and which the CHANGELOG had claimed on the strength of a throwaway
golden rather than a committed test; explicit-survey df reaching every level;
group df independent of event_study_df; the mutation, read-only, 0-d and
off-vocabulary regressions. The CHANGELOG claim now describes what ships, and
bootstrap replay has a real TODO.md row instead of an untracked "tracked
follow-up".
…, pretrends, plot_event_study)
Local review round 2 (round 1 findings all cleared; no P0/P1 this round). Data minimization. The aggregation kit retained ``all_units`` and ``unit_to_idx`` verbatim, so a fitted CS result - which is picklable, and which users share as an artifact - became a carrier for raw unit identifiers. Those are routinely names, emails or administrative IDs, and NO pre-existing public field on CallawaySantAnnaResults carried them, so this branch would have been the regression that introduced the disclosure. The kit needs only POSITION: the influence arrays index by treated_idx/control_idx, and the fast aggregation path keys off object identity plus length, never a label lookup. Verified by substituting canonical codes on a fitted result and re-aggregating - identical at atol=rtol=0, not merely close - so the kit now stores 0..n-1 codes. ``unit_cohorts`` is deliberately kept: those are first-treatment PERIODS, reported as group labels, not identifiers. Pinned by a sentinel-identifier test that searches the whole pickled artifact rather than spot-checking the kit. summary(alpha=) printed "Confidence intervals at alpha=<passed value>" while the interval had been computed at self.alpha and is never recomputed - false provenance on a display path. EventStudyResults.summary already solved this by raising when the passed alpha differs; this was another divergence from the sibling, and now matches it. Inertness coverage extended to the designs the registry gives their own weighting, reference-cell and VCV-index semantics, none of which the balanced-panel fixture reached: base_period="universal", control_group= "not_yet_treated" with anticipation, allow_unbalanced_panel=True, and survey parity on the ESTIMATES rather than only the df metadata. 62 contract tests (up from 47); 1136 pass across the staggered, SDDD, survey, methodology, event-study-surface, honest-did, pretrends, ledger and docs suites, plus 491 across the sibling estimators that share the aggregation module.
Local review round 3. The previous round introduced N_KIND_VOCABULARY and documented it as SHARED by every count-bearing container, then validated it on AggregationResult only - a contract declared in one place and enforced in one of its two implementations. A public EventStudyResults could still carry and serialize n_kind="widgets", which defeats exactly the cross-container routing the shared vocabulary exists to enable, and does so through the unchecked side. EventStudyResults.__post_init__ now applies the same membership check. Verified non-breaking before adding it: the values shipped producers actually emit are "groups", "switcher_cells", "cells", "units" and "obs" (plus None), every one of which is already in the vocabulary, so no producer changes behavior. The regression test asserts both directions on both containers - an unknown value raises, and every real producer value still constructs - rather than only the rejection, since a vocabulary that quietly stopped accepting a live producer value would be the worse failure.
Local review round 4 (no P0/P1; both remaining P2s resolved). n_kind on simple aggregation was hardcoded "units", but fit() only counts units on a panel: with panel=False it counts ROWS, because a repeated cross section has no unit tracking. Reporting that total as "units" misdescribes the sample through the very routing key the shared vocabulary exists to make trustworthy - and it is the conflation N_KIND_VOCABULARY's own documentation warns against. It now reads is_panel from the retained kit, which already carried it. Estimates and inference were never affected; only the label was. The regression pins BOTH directions, so this cannot degrade into a blanket rename: RCS reports "obs" (and the count matches the fitted totals) while the panel fixture still reports "units". The public fit() docstring still presented aggregate= and balance_e= as ordinary parameters. Anyone reading the API reference had no way to know they now emit a FutureWarning and are removed at 4.0. Both are marked deprecated with their ledger rows (M-020, M-117) and point at the post-fit call. The aggregate= entry also records WHY the fit-time path still exists - it is the temporary compatibility surface for compute_honest_did, compute_pretrends_power and plot_event_study - so the docs do not read as recommending a path that is being removed. 64 contract tests; 1200 pass across the staggered, SDDD, survey, methodology, event-study-surface, honest-did, pretrends, ledger and docs suites. Unrelated, NOT introduced here: tests/test_wooldridge.py:: TestMethodologyCorrectness::test_ols_etwfe_att_matches_callaway_santanna fails identically at the pre-branch base cba7258 (ETWFE -0.0431 vs CS -0.0261). It is a pre-existing main-branch parity failure and is left for its own change.
…ccounting A resource claim that names only the dominant payload reads as complete when it is not. The accounting now also records the replicate-weight survey matrix (O(n_units x n_replicates)), states that the O(n_units) bookkeeping bound is panel-only because several RCS arrays are observation-length, and records that raw unit identifiers are deliberately not retained.
Overall assessmentExecutive summary
MethodologyP1 — Bootstrap group results retain false analytical-DF provenanceLocation: Impact: Concrete fix: When bootstrap statistics replace a group row, set P3 — Documented SDDD aggregation deviationsLocation: Impact: Eligible-treated cohort weights, the R group-WIF difference, and the distinction between default simple ATT and paper Eq. 4.14 are explicitly documented. They are not defects. Concrete fix: None required. Code QualityNo additional findings. PerformanceNo findings. The retained MaintainabilityNo unmitigated findings. Tech DebtP3 — Tracked deferred integrationsLocation: Impact: Bootstrap post-fit aggregation raises, and the new event-study container is not yet accepted by HonestDiD, PreTrendsPower, or plotting. Both limitations fail closed. Concrete fix: No action required in this PR; follow the referenced SecurityNo findings. Raw unit identifiers are replaced with positional codes, and survey cluster/strata values are factorized before retention. Documentation/TestsP2 — Primary API and LLM guides still advertise the deprecated fit-time APILocation: Impact: The headline new Concrete fix: Add Path to Approval
|
…ggregate()
CI round 1 (Mypy gate + codex review).
P1 - bootstrap group rows kept false analytical df provenance. The bootstrap
block replaces each group row's se/p_value/conf_int with percentile values but
left the df_used this branch added, so group_effects[g] claimed a t-reference
had governed inference that never used one. The event-study path immediately
below already clears its df for exactly this reason - the same clearing rule
was written for one surface and not its twin, the third instance of that
pattern in this branch. Reproduced on a survey bootstrap fit (event_study_df
None, group df_used 15.0) before fixing, and fixed in BOTH estimators:
CallawaySantAnna and StaggeredTripleDifference, which carries its own copy of
the replacement loop. group_effects is a public field, so the wrong value was
reachable even though aggregate("group") itself fails closed on bootstrap
fits. Regression covers clustered AND explicit-survey bootstrap.
Mypy (6 errors, all introduced here; verified against the workflow's exact
pins - mypy 2.3.0 / numpy 2.4.5 / pandas 3.0.3 / scipy 1.17.1 - since this
worktree's own venv cannot run mypy):
- Three "Any | None is not indexable" in staggered_aggregation: relaxing df to
Optional left the no-bookkeeping fallback branches indexing a possibly-None
frame. Each now raises explicitly when neither the kit nor the frame is
available, which both narrows the type and states the invariant.
- Two incompatible-base-class errors: the bootstrap mixin's TYPE_CHECKING stub
for _compute_combined_influence_function still declared df/unit as required,
contradicting the aggregation mixin it shadows wherever both are mixed in.
- _aggregation_kit was set dynamically on the results object; it is now a
declared field (repr=False, compare=False) and therefore part of the typed
contract rather than an undeclared attribute.
P2 - the headline method was undocumented. aggregate() is added to the results
autosummary in docs/api/staggered.rst and to both agent-facing guides, which
still presented fit-time aggregate=/balance_e= with no deprecation context.
The guides now lead with the post-fit call and keep the fit-time form only
where it is still required (plot_event_study, compute_honest_did,
compute_pretrends_power), labelled as the compatibility path.
66 contract tests; 1202 pass across the affected suites; mypy clean.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0/P1 findings. Executive summary
MethodologyP3 — Prior bootstrap provenance finding resolvedLocation: Impact: Bootstrap group statistics now correctly clear analytical Concrete fix: None required. P3 — Documented SDDD deviationsLocation: Impact: Eligible-treated weighting, group WIF behavior, and the Eq. 4.14 distinction are explicitly documented and match the paper review. Concrete fix: None required. Code QualityNo findings. PerformanceNo findings. Retained influence-function and survey-design memory costs are documented. MaintainabilityNo findings. Tech DebtP3 — Tracked deferred integrationsLocation: Impact: Bootstrap post-fit aggregation, downstream event-study adapters, and calendar aggregation remain unavailable but fail closed. Concrete fix: None required in this PR. SecurityNo findings. Raw unit identifiers are replaced with positional codes in the retained kit. Documentation/TestsP2 — Guide’s post-fit example always raisesLocation: Impact: The example fits with Concrete fix: Omit P2 — SDDD provenance fix lacks a regression testLocation: Impact: Tests cover clustered and survey Callaway–Sant’Anna fits but not the corresponding SDDD branch. Concrete fix: Add an explicit-survey SDDD bootstrap test with Validation was limited to static parsing because NumPy and pytest are unavailable in the review environment. |
…DD provenance regression CI round 2 (rerun: no unmitigated P0/P1; both remaining P2s resolved). The post-fit example added last round constructed the estimator with n_bootstrap=999 and then called results.aggregate() - which raises NotImplementedError on a bootstrapped fit, by the very contract documented a few lines below it. Reproduced before fixing. The analytical example now omits n_bootstrap and says why, and the fit-time form is presented as the required path for BOTH cases that need it: bootstrap inference, and the downstream consumers that still read the fit-time surface. SDDD regression for the bootstrap df-provenance fix. The previous round fixed both estimators but tested only CallawaySantAnna - the same fix-one-twin gap that produced the bug. Verified the new test is not vacuous by reverting the fix and confirming it fails. That check also exposed an asymmetry now recorded in the test rather than papered over: only the explicit-survey configuration exercises the clearing. SDDD's plain-cluster path never resolves a finite analytical df, so df_used is already None there and the non-survey parametrization passes either way; it is kept as a cheap guard against that changing, not as a reproduction. The CallawaySantAnna equivalent was checked the same way and DOES fail in both configurations, so its coverage claim stands. 68 contract tests; 1204 pass across the affected suites; mypy clean against the workflow's pins.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment✅ Looks good — no unmitigated P0/P1 findings. Executive summary
MethodologyP3 — Bootstrap provenance resolvedImpact: Bootstrap group rows correctly clear analytical Location: Concrete fix: None required. P3 — Documented SDDD deviationsImpact: Eligible-treated weighting, group WIF behavior, and Eq. 4.14 semantics match the Registry’s documented deviations. Location: Concrete fix: None required. Code QualityNo findings. PerformanceNo findings. Retained-state memory costs are documented. MaintainabilityNo findings. Tech DebtP3 — Tracked limitationsImpact: Bootstrap re-aggregation, downstream adapters, and calendar aggregation remain unavailable but fail closed. Location: Concrete fix: None required in this PR. SecurityNo findings. Survey identifiers are factorized before retention, and no secrets were detected. Documentation/TestsP3 — Prior findings resolvedImpact: The guide now uses an analytical fit for post-fit aggregation, and SDDD bootstrap DF clearing has regression coverage. Location: Concrete fix: None required. Runtime tests could not run because NumPy and pytest are unavailable; all 12 changed Python files passed static AST parsing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 2b PR 1 of the 4.0 program: the post-fit aggregation contract and its first consumer.
results.aggregate(type, weights=None, *, balance_e=None)onCallawaySantAnna.aggregate("event_study")returns the unifiedEventStudyResults— giving [M-092]'s exported-but-unreachable container a public producer at last;"simple"/"group"return the newAggregationResult. Returns a new object; the parent is unchanged.fit(aggregate=)/fit(balance_e=)deprecated behind a sentinel default, so a plainfit()never warns. The shimmed path still returns the fully populated legacy surface its consumers read.self; it now returns all five as anEventStudyAggregation. That is what makes re-aggregation possible without mutating the result.StaggeredTripleDifferenceinherits the same mixin and is the only reader of the Eq. 4.14 overall, so it moved in the same change.fit()(nothing it needs survives the call). It holds no data matrices and no source panel.planned→shimmedand gains atest_ref(it carried none); M-117 (balance_eontoaggregate()) and M-122 (AggregationResult) are new.Spec correction. §6 claimed "CallawaySantAnna already stores them" of influence functions. It does not — the field was declared and never assigned — so retention is work in every migrating PR, not just where "missing". Corrected in place.
Methodology references
did::aggte.safe_inference_batch.aggregate("calendar")raises: CS implements no calendar aggregator, and the DEFERRED row stands.Validation
aggregate()reproduces the fit-time numbers atatol=rtol=1e-14for every supported type, across a balanced panel, repeated cross-sections, abalance_esweep,anticipation=1,base_period="universal",control_group="not_yet_treated"with anticipation,allow_unbalanced_panel=True, and an explicit PSU survey design.tests/test_aggregate_contract.py— new, 64 tests, thetest_reffor M-020 / M-117 / M-122: inertness, immutability across mixed-order and repeated calls, the kit holding no DataFrame and surviving pickle, the whole fail-closed surface, schema/n_kind/weightpinning, the shim warning only when passed, and a StaggeredTripleDifference regression for the Eq. 4.14 overall.EventStudyResultsproducer suite (staggered, SDDD, survey, methodology, event-study-surface, honest-did, pretrends, ledger, docs-IA, doc-deps, and the sibling estimators sharing the aggregation module).Known limitations — fail-closed and tracked, not silent
aggregate()on a bootstrapped fit fails rather than substituting analytical inference. The value-boundBootstrapReplaySpecis in-tree and spike-verified (bit-identical replay, picklable, immune to post-fitset_params); wiring it is aTODO.mdrow.EventStudyResultsis not yet accepted bycompute_honest_did,compute_pretrends_powerorplot_event_study— all three dispatch onCallawaySantAnnaResults. Their error messages say so explicitly rather than pointing at a path that raises, andfit(aggregate=)remains the working route for them through 4.0. Adapters plusbase_period/anticipationprovenance are aTODO.mdrow.Note on a local-only test failure (does NOT affect this PR)
tests/test_wooldridge.py::TestMethodologyCorrectness::test_ols_etwfe_att_matches_callaway_santannafails in one maintainer's local checkout but is green in CI, including at this branch's base. It is unrelated to this PR, which touches none of that code.The cause is environmental, and worth recording because it hides a real issue:
load_mpdta()fetches its dataset over the network and, on failure, silently falls back to synthetic_construct_mpdta_data(). That upstream URL now returns 404, so a fresh CI runner (empty cache) always gets the synthetic substitute, while a machine holding a stale cache gets the real data. The two are structurally identical — same shape, cohorts and unit count — so the swap is invisible; only the values differ.Consequently the assertion passes on synthetic data (max |ETWFE−CS| = 0.0017) and fails on the real dataset (0.0171, against
atol=5e-3). Tracked separately; nothing here depends on those loaders.Security / privacy
0..n-1codes rather than raw unit identifiers, so a shared/pickled results artifact cannot carry names, emails or administrative IDs. Verified numerically inert atatol=rtol=0and pinned by a sentinel test that searches the whole serialized artifact.