Skip to content

Commit 98ce01e

Browse files
igerberclaude
andcommitted
refactor(survey): consolidate panel-to-unit collapse (ContinuousDiD/EfficientDiD)
Extract the row_idx->unit survey-design collapse hand-rolled at four sites (continuous_did.py event-study/overall/bootstrap SE + efficient_did.py build-once) into two diff_diff/survey.py helpers: - ResolvedSurveyDesign.subset_to_units_by_row_idx(row_idx, unit_weights=None): folds the index-and-recount preamble around the existing subset_to_units. - build_unit_first_row_index(unit_values, unit_order): first-panel-row index per unit (replaces ContinuousDiD's slow df.iterrows() build and EfficientDiD's inline .values scan). Bit-identical: same np.unique + subset_to_units + arrays; survey-weighted SEs, df_survey, and design-effect metadata are unchanged (oracle test locks it vs the old inline logic). Removes ~95 lines of duplicated collapse. StackedDiD is deliberately left on its own path (control units are duplicated across sub-experiments, so it re-resolves at stacked granularity rather than collapsing to one row per unit) with a clarifying comment; the residual stacked-specific dedup is parked in TODO.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 60abfb0 commit 98ce01e

7 files changed

Lines changed: 222 additions & 107 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3434
still defers cluster with a warning).
3535

3636
### Changed
37+
- **Internal: consolidated the panel-to-unit survey-design collapse** shared by `ContinuousDiD`
38+
and `EfficientDiD` into two `diff_diff/survey.py` helpers —
39+
`ResolvedSurveyDesign.subset_to_units_by_row_idx` (folds the index-and-recount preamble around
40+
the existing `subset_to_units`) and `build_unit_first_row_index` (first-panel-row index per
41+
unit). Removes four hand-rolled copies of the collapse plus a slow `df.iterrows()` index build.
42+
No behavior change: survey-weighted SEs, `df_survey`, and design-effect metadata are
43+
bit-identical (locked by an oracle test against the old inline logic).
3744
- **`DiagnosticReport` now routes `SpilloverDiDResults`.** Previously a fitted
3845
`SpilloverDiD` (Butts 2021) result matched no `_APPLICABILITY` / `_PT_METHOD` entry, so
3946
`DiagnosticReport(spillover_result)` reported every check as `not_applicable`. It now routes

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ The `Origin` column (Actionable tables) and the `PR` column (Deferred tables) bo
2828

2929
| Issue | Location | Origin | Effort | Priority |
3030
|-------|----------|--------|--------|----------|
31-
| Survey-design resolution / collapse patterns are inconsistent across panel estimators — `ContinuousDiD` rebuilds unit-level design in SE code, `EfficientDiD` builds once in `fit()`, `StackedDiD` re-resolves on stacked data. Extract shared helpers for panel-to-unit collapse, post-filter re-resolution, and metadata recomputation. | `continuous_did.py`, `efficient_did.py`, `stacked_did.py` | #226 | Mid | Low |
3231
| `SyntheticControl` remaining ADH-2015 §4 items: the regression-weight `W^reg = X_0'(X_0 X_0')^{-1} X_1` extrapolation diagnostic (flag implied OLS weights outside `[0,1]`) and sparse-SC subset search (`l < J`, holding `V` fixed). LOO, in-time placebo, CV `V`-selection, and inverse-variance `V` have landed; these two are the deferred tail. | `synthetic_control.py`, `synthetic_control_results.py` | ADH-2015 | Mid | Low |
3332
| `SyntheticControl` conformal (CWZ 2021) extensions: (a) one-sided / signed-`t` variants (§7); (b) covariates in the conformal proxy (`X_jt`, eqs 4/6 — current proxy is outcomes-only); (c) AR / innovation-permutation path (Lemmas 5-7) for time-series proxies. The joint test, pointwise CIs, and average-effect CI have landed. | `conformal.py`, `synthetic_control_results.py` | CWZ-2021 | Heavy | Low |
3433
| `ContinuousDiD` CGBS-2024 extensions (matches R `contdid` v0.1.0 deferral set): (a) `covariates=` kwarg; (b) discrete-treatment saturated regression (integer dose currently warned, not routed to per-level coefficients); (c) lowest-dose-as-control per Remark 3.1 when `P(D=0)=0`. REGISTRY `## ContinuousDiD` → Implementation Checklist marks these `[ ]`. | `continuous_did.py` | CGBS-2024 | Heavy | Low |
@@ -123,6 +122,7 @@ Doable in principle, but no current caller and/or explicitly out of paper scope.
123122

124123
| Issue | Location | PR | Priority |
125124
|-------|----------|----|----------|
125+
| `StackedDiD` survey re-resolution intra-file dedup (raw-weight extraction ×3, compose-normalize ×3, resolve-on-stacked ×2). The cross-estimator ContinuousDiD/EfficientDiD panel-to-unit collapse consolidation LANDED (#226 shared helpers `ResolvedSurveyDesign.subset_to_units_by_row_idx` / `build_unit_first_row_index`); StackedDiD is deliberately NOT on that path (control units are duplicated across sub-experiments, so it re-resolves at stacked granularity rather than collapsing to one row per unit). The residual is stacked-specific, low value, and touches the numerically-sensitive composed-weight renormalization. Post-filter re-resolution / metadata-recompute unification across the three estimators was assessed and is not warranted — they use genuinely different mechanisms and already delegate to shared `_resolve_survey_for_fit` / `compute_survey_metadata`. | `stacked_did.py` | #226 | Low |
126126
| dCDH parity-test SE/CI assertions only cover pure-direction scenarios; mixed-direction SE comparison is structurally apples-to-oranges (cell-count vs obs-count weighting). | `test_chaisemartin_dhaultfoeuille_parity.py` | #294 | Low |
127127
| `HeterogeneousAdoptionDiD` survey-design API consolidation (**scheduled: next minor bump**): drop the deprecated `survey=` / `weights=` kwargs on all 8 HAD surfaces; only `survey_design=` remains. Also fold the legacy back-end `weights=` routing into the unified `_resolve_survey_for_fit` path. DeprecationWarning has shipped; the removal is ~50 LoC gated on the semver bump. | `had.py`, `had_pretests.py` | next minor | Medium |
128128
| `HeterogeneousAdoptionDiD` joint cross-horizon covariance / sup-t bands: per-horizon SEs use independent sandwiches (paper-faithful pointwise CIs per Pierce-Schott Fig 2). Follow-ups (low demand): IF-based stacking for joint cross-horizon inference; analytical H×H covariance on the weighted ES path; a sup-t band on the unweighted ES path. | `had.py::_fit_event_study` | Phase 2b / 4.5 B | Low |

diff_diff/continuous_did.py

Lines changed: 46 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@
3131
)
3232
from diff_diff.linalg import _rank_guarded_inv, solve_ols
3333
from diff_diff.survey import (
34-
ResolvedSurveyDesign,
3534
_resolve_survey_for_fit,
3635
_validate_unit_constant_survey,
36+
build_unit_first_row_index,
3737
compute_survey_vcov,
3838
)
3939
from diff_diff.utils import safe_inference
@@ -413,8 +413,7 @@ def fit(
413413

414414
# Filter out NaN cells (e.g., from zero effective survey mass)
415415
gt_results = {
416-
gt: r for gt, r in gt_results.items()
417-
if np.isfinite(r.get("att_glob", np.nan))
416+
gt: r for gt, r in gt_results.items() if np.isfinite(r.get("att_glob", np.nan))
418417
}
419418

420419
if len(gt_results) == 0:
@@ -573,9 +572,12 @@ def fit(
573572
# Survey df for t-distribution inference (unit-level, not panel-level)
574573
_survey_df = analytic.get("df_survey")
575574
# Guard: replicate design with undefined df → NaN inference
576-
if (_survey_df is None and resolved_survey is not None
577-
and hasattr(resolved_survey, 'uses_replicate_variance')
578-
and resolved_survey.uses_replicate_variance):
575+
if (
576+
_survey_df is None
577+
and resolved_survey is not None
578+
and hasattr(resolved_survey, "uses_replicate_variance")
579+
and resolved_survey.uses_replicate_variance
580+
):
579581
_survey_df = 0
580582

581583
# Recompute survey_metadata from unit-level design so reported
@@ -589,8 +591,7 @@ def fit(
589591

590592
# Propagate replicate df override to survey_metadata for display
591593
# (but not the df=0 sentinel — keep metadata as None for undefined df)
592-
if (_survey_df is not None and _survey_df != 0
593-
and survey_metadata is not None):
594+
if _survey_df is not None and _survey_df != 0 and survey_metadata is not None:
594595
if survey_metadata.df_survey != _survey_df:
595596
survey_metadata.df_survey = _survey_df
596597

@@ -624,30 +625,8 @@ def fit(
624625
unit_resolved_es = None
625626
if resolved_survey is not None:
626627
row_idx = precomp["unit_first_panel_row"]
627-
uw = (
628-
precomp.get("unit_survey_weights")
629-
if precomp.get("unit_survey_weights") is not None
630-
else np.ones(n_units)
631-
)
632-
us = (
633-
resolved_survey.strata[row_idx]
634-
if resolved_survey.strata is not None
635-
else None
636-
)
637-
up = (
638-
resolved_survey.psu[row_idx]
639-
if resolved_survey.psu is not None
640-
else None
641-
)
642-
uf = (
643-
resolved_survey.fpc[row_idx]
644-
if resolved_survey.fpc is not None
645-
else None
646-
)
647-
n_strata_u = len(np.unique(us)) if us is not None else 0
648-
n_psu_u = len(np.unique(up)) if up is not None else 0
649-
unit_resolved_es = resolved_survey.subset_to_units(
650-
row_idx, uw, us, up, uf, n_strata_u, n_psu_u,
628+
unit_resolved_es = resolved_survey.subset_to_units_by_row_idx(
629+
row_idx, unit_weights=precomp.get("unit_survey_weights")
651630
)
652631

653632
for e_val, info_e in event_study_effects.items():
@@ -711,13 +690,21 @@ def fit(
711690

712691
# Score-scale: psi = w * if_es (matches TSL bread)
713692
psi_es = unit_resolved_es.weights * if_es
714-
variance, _nv = compute_replicate_if_variance(psi_es, unit_resolved_es)
715-
es_se = float(np.sqrt(max(variance, 0.0))) if np.isfinite(variance) else np.nan
693+
variance, _nv = compute_replicate_if_variance(
694+
psi_es, unit_resolved_es
695+
)
696+
es_se = (
697+
float(np.sqrt(max(variance, 0.0)))
698+
if np.isfinite(variance)
699+
else np.nan
700+
)
716701
else:
717702
X_ones_es = np.ones((n_units, 1))
718703
tsl_scale_es = float(unit_resolved_es.weights.sum())
719704
if_es_tsl = if_es * tsl_scale_es
720-
vcov_es = compute_survey_vcov(X_ones_es, if_es_tsl, unit_resolved_es)
705+
vcov_es = compute_survey_vcov(
706+
X_ones_es, if_es_tsl, unit_resolved_es
707+
)
721708
es_se = float(np.sqrt(np.abs(vcov_es[0, 0])))
722709
else:
723710
es_se = float(np.sqrt(np.sum(if_es**2)))
@@ -831,15 +818,11 @@ def _precompute_structures(
831818
unit_cohorts[i] = unit_first.loc[u, first_treat]
832819
dose_vector[i] = unit_first.loc[u, dose]
833820

834-
# Build unit-to-first-panel-row mapping (for subsetting panel-level arrays)
835-
# This maps each unit index to the positional index of its first row in df.
836-
unit_first_panel_row = np.zeros(n_units, dtype=int)
837-
seen_units: set = set()
838-
for pos_idx, (_, row) in enumerate(df.iterrows()):
839-
u = row[unit]
840-
if u not in seen_units:
841-
seen_units.add(u)
842-
unit_first_panel_row[unit_to_idx[u]] = pos_idx
821+
# Build unit-to-first-panel-row mapping (for subsetting panel-level
822+
# arrays): the positional index of each unit's first row in df, aligned
823+
# to ``all_units`` (== ``unit_to_idx`` order since
824+
# ``unit_to_idx = {u: i for i, u in enumerate(all_units)}``).
825+
unit_first_panel_row = build_unit_first_row_index(df[unit].values, all_units)
843826

844827
# Per-unit survey weights (take first obs per unit from panel data)
845828
unit_survey_weights = None
@@ -949,8 +932,10 @@ def _compute_dose_response_gt(
949932
# Guard against zero effective mass (e.g., after subpopulation)
950933
if np.sum(w_treated) <= 0 or np.sum(w_control) <= 0:
951934
return {
952-
"att_glob": np.nan, "acrt_glob": np.nan,
953-
"n_treated": 0, "n_control": 0,
935+
"att_glob": np.nan,
936+
"acrt_glob": np.nan,
937+
"n_treated": 0,
938+
"n_control": 0,
954939
"att_d": np.full(len(dvals), np.nan),
955940
"acrt_d": np.full(len(dvals), np.nan),
956941
}
@@ -1293,23 +1278,8 @@ def _compute_analytical_se(
12931278
# but influence functions are unit-level (n_units). Build a unit-level
12941279
# ResolvedSurveyDesign by subsetting to one obs per unit.
12951280
row_idx = precomp["unit_first_panel_row"]
1296-
unit_weights = precomp.get("unit_survey_weights")
1297-
if unit_weights is None:
1298-
unit_weights = np.ones(n_units)
1299-
1300-
unit_strata = (
1301-
resolved_survey.strata[row_idx] if resolved_survey.strata is not None else None
1302-
)
1303-
unit_psu = resolved_survey.psu[row_idx] if resolved_survey.psu is not None else None
1304-
unit_fpc = resolved_survey.fpc[row_idx] if resolved_survey.fpc is not None else None
1305-
1306-
# Count unique strata/PSU in the unit-level subset
1307-
n_strata_unit = len(np.unique(unit_strata)) if unit_strata is not None else 0
1308-
n_psu_unit = len(np.unique(unit_psu)) if unit_psu is not None else 0
1309-
1310-
unit_resolved = resolved_survey.subset_to_units(
1311-
row_idx, unit_weights, unit_strata, unit_psu, unit_fpc,
1312-
n_strata_unit, n_psu_unit,
1281+
unit_resolved = resolved_survey.subset_to_units_by_row_idx(
1282+
row_idx, unit_weights=precomp.get("unit_survey_weights")
13131283
)
13141284

13151285
X_ones = np.ones((n_units, 1))
@@ -1370,7 +1340,11 @@ def _rep_se(if_vals):
13701340

13711341
# Return unit-level survey df and resolved design for metadata recomputation
13721342
# Only override with n_valid-based df when replicates were actually dropped
1373-
if resolved_survey is not None and hasattr(resolved_survey, 'uses_replicate_variance') and resolved_survey.uses_replicate_variance:
1343+
if (
1344+
resolved_survey is not None
1345+
and hasattr(resolved_survey, "uses_replicate_variance")
1346+
and resolved_survey.uses_replicate_variance
1347+
):
13741348
if _rep_n_valid < unit_resolved.n_replicates:
13751349
unit_df_survey = _rep_n_valid - 1 if _rep_n_valid > 1 else None
13761350
else:
@@ -1415,7 +1389,11 @@ def _run_bootstrap(
14151389

14161390
# Reject replicate-weight designs for bootstrap — replicate variance
14171391
# is an analytical alternative to bootstrap, not compatible with it
1418-
if resolved_survey is not None and hasattr(resolved_survey, "uses_replicate_variance") and resolved_survey.uses_replicate_variance:
1392+
if (
1393+
resolved_survey is not None
1394+
and hasattr(resolved_survey, "uses_replicate_variance")
1395+
and resolved_survey.uses_replicate_variance
1396+
):
14191397
raise NotImplementedError(
14201398
"ContinuousDiD bootstrap (n_bootstrap > 0) is not supported "
14211399
"with replicate-weight survey designs. Replicate weights provide "
@@ -1429,22 +1407,9 @@ def _run_bootstrap(
14291407
# Build unit-level ResolvedSurveyDesign for survey-aware bootstrap
14301408
unit_resolved = None
14311409
if resolved_survey is not None:
1432-
from diff_diff.survey import ResolvedSurveyDesign
1433-
14341410
row_idx = precomp["unit_first_panel_row"]
1435-
unit_weights = precomp.get("unit_survey_weights")
1436-
if unit_weights is None:
1437-
unit_weights = np.ones(n_units)
1438-
unit_strata = (
1439-
resolved_survey.strata[row_idx] if resolved_survey.strata is not None else None
1440-
)
1441-
unit_psu = resolved_survey.psu[row_idx] if resolved_survey.psu is not None else None
1442-
unit_fpc = resolved_survey.fpc[row_idx] if resolved_survey.fpc is not None else None
1443-
n_strata_u = len(np.unique(unit_strata)) if unit_strata is not None else 0
1444-
n_psu_u = len(np.unique(unit_psu)) if unit_psu is not None else 0
1445-
unit_resolved = resolved_survey.subset_to_units(
1446-
row_idx, unit_weights, unit_strata, unit_psu, unit_fpc,
1447-
n_strata_u, n_psu_u,
1411+
unit_resolved = resolved_survey.subset_to_units_by_row_idx(
1412+
row_idx, unit_weights=precomp.get("unit_survey_weights")
14481413
)
14491414

14501415
# Generate bootstrap weights — PSU-level when survey design is present
@@ -1682,7 +1647,7 @@ def _bootstrap_gt_cell(gt, info):
16821647
boot_es[e],
16831648
alpha=self.alpha,
16841649
context=f"event study e={e}",
1685-
)
1650+
)
16861651
es_se[e] = se_e
16871652
es_ci[e] = ci_e
16881653
es_p[e] = p_e

diff_diff/efficient_did.py

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ def fit(
521521
)
522522

523523
# Resolve survey design if provided
524-
from diff_diff.survey import _resolve_survey_for_fit
524+
from diff_diff.survey import _resolve_survey_for_fit, build_unit_first_row_index
525525

526526
resolved_survey, survey_weights, survey_weight_type, survey_metadata = (
527527
_resolve_survey_for_fit(survey_design, data, "analytical")
@@ -655,33 +655,14 @@ def fit(
655655
# order. The previous approach (groupby cumcount == 0) yielded
656656
# first-appearance order which can differ from sorted order when the
657657
# input DataFrame is not pre-sorted by unit.
658-
first_pos: Dict[Any, int] = {}
659-
for i, u in enumerate(df[unit].values):
660-
if u not in first_pos:
661-
first_pos[u] = i
662-
self._unit_first_panel_row = np.array([first_pos[u] for u in all_units])
658+
self._unit_first_panel_row = build_unit_first_row_index(df[unit].values, all_units)
663659

664660
# Build unit-level ResolvedSurveyDesign once (avoids repeated
665661
# construction in _compute_survey_eif_se and ensures consistent
666662
# unit-level df for safe_inference t-distribution).
667663
if resolved_survey is not None:
668-
row_idx = self._unit_first_panel_row
669-
unit_weights_s = resolved_survey.weights[row_idx]
670-
unit_strata = (
671-
resolved_survey.strata[row_idx] if resolved_survey.strata is not None else None
672-
)
673-
unit_psu = resolved_survey.psu[row_idx] if resolved_survey.psu is not None else None
674-
unit_fpc = resolved_survey.fpc[row_idx] if resolved_survey.fpc is not None else None
675-
n_strata_u = len(np.unique(unit_strata)) if unit_strata is not None else 0
676-
n_psu_u = len(np.unique(unit_psu)) if unit_psu is not None else 0
677-
self._unit_resolved_survey = resolved_survey.subset_to_units(
678-
row_idx,
679-
unit_weights_s,
680-
unit_strata,
681-
unit_psu,
682-
unit_fpc,
683-
n_strata_u,
684-
n_psu_u,
664+
self._unit_resolved_survey = resolved_survey.subset_to_units_by_row_idx(
665+
self._unit_first_panel_row
685666
)
686667
# Use unit-level df (not panel-level) for t-distribution
687668
self._survey_df = self._unit_resolved_survey.df_survey

diff_diff/stacked_did.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,10 @@ def _refit_stacked(w_r):
773773

774774
# Re-resolve survey design on the stacked data so that strata/PSU
775775
# arrays have the correct length for TSL variance estimation.
776+
# (Unlike ContinuousDiD/EfficientDiD, StackedDiD does NOT collapse
777+
# to unit level via ``subset_to_units_by_row_idx``: control units are
778+
# duplicated across sub-experiments, so the design must be resolved
779+
# at the stacked granularity, not one row per original unit.)
776780
resolved_stacked = survey_design.resolve(stacked_df)
777781

778782
# Create a copy with composed weights (normalized to sum=n_stacked)

0 commit comments

Comments
 (0)