Skip to content

Commit 2293ba1

Browse files
authored
Merge pull request #544 from igerber/fix/ddd-power-panel-routing
fix: route TripleDifference power to panel DGP when n_periods > 2
2 parents 531db01 + e059454 commit 2293ba1

6 files changed

Lines changed: 397 additions & 25 deletions

File tree

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212

1313
### Fixed
1414
- **`LinearRegression.get_se()` / `get_inference()` no longer return a `NaN` standard error from a tiny-negative variance artifact.** A high-leverage / degenerate coefficient (e.g. an absorbed-FE dummy near-collinear with the treatment, whose Bell-McCaffrey Satterthwaite DOF already hits the noise-floor guard) can have a CR2/HC variance of ~0 (≈1e-32) whose vcov diagonal lands just-below-zero under BLAS-dependent float rounding; `np.sqrt` of the negative then produced a `NaN` SE **nondeterministically** — passing single-threaded but failing under the parallel pure-Python full-suite run (`tests/test_methodology_wls_cr2.py::TestLinearRegressionFENanGuardEndToEnd::test_did_absorbed_fe_lr_inference_nan_for_guarded_coefs`). Both SE sites now clamp the vcov diagonal at 0, so the SE is finite (0 for a genuinely-zero variance), deterministic, and BLAS-independent. **No change for any positive variance** (the clamp is a no-op there); only the previously-`NaN` degenerate case is affected.
15+
- **`TripleDifference` power analysis now honors `n_periods > 2`.** `simulate_power`,
16+
`simulate_mde`, and `simulate_sample_size` previously routed DDD to the
17+
cross-sectional 2×2×2 `generate_ddd_data` regardless of `n_periods` (emitting an
18+
"n_periods ignored" warning). They now route to the panel DGP
19+
`generate_ddd_panel_data` when `n_periods > 2`, honoring `n_periods`/`treatment_period`
20+
and sizing the panel by `n_units` directly (the sample-size search switches from the
21+
multiple-of-8 grid to a continuous step-1 search). Because `simulate_power` defaults
22+
to `n_periods=4`, the default DDD power call now uses the panel DGP. The panel DGP has
23+
within-unit serial correlation, so construct the estimator as
24+
`TripleDifference(cluster="unit")` for valid power — a `UserWarning` fires otherwise.
25+
`treatment_fraction` remains inert (balanced 2×2×2); pass `group_frac`/`partition_frac`
26+
via `data_generator_kwargs`. See `docs/methodology/REGISTRY.md` §PowerAnalysis.
1527

1628
## [3.5.2] - 2026-06-08
1729

TODO.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ Deferred items from PR reviews that were not addressed before merge.
9393
| Survey sandwich SE is not exactly invariant to zero-weight (subpopulation / padded) rows: the shared `_compute_stratified_psu_meat` finite-sample correction counts zero-weight units as PSUs (an `n_psu/(n_psu-1)`-style factor), so adding zero-weight rows shifts the SE by a second-order amount (~2e-4 relative in the EfficientDiD e2e). The point estimate is exactly invariant and the weighted scores of zero-weight rows are already zero — only the DOF correction's PSU count includes them. Cross-cutting across all survey-enabled estimators; fix by counting only positive-weight PSUs in the correction. | `survey.py` (`_compute_stratified_psu_meat`) | PR-B follow-up | Low |
9494
| ImputationDiD: leave-one-out (LOO) conservative-variance refinement (BJS 2024 Supplementary Appendix A.9) not implemented — a finite-sample improvement to the auxiliary-model residuals that reduces overfitting of `tau_tilde_g` to `epsilon`. The asymptotic Theorem-3 variance is implemented and matches R `didimputation` (which also omits LOO by default). | `imputation.py` | imputation-validation follow-up | Low |
9595
| TROP: extend Wave 4's `_setup_trop_data` helper to also cover the duplicated bootstrap resampling loop in `_bootstrap_variance` / `_bootstrap_variance_global` (~40 LoC dedup; mirrors the data-setup helper pattern with a `fit_callable` parameter for the per-draw refit step). | `trop_local.py`, `trop_global.py` | follow-up | Low |
96-
| TripleDifference power auto-routing: `power.simulate_power` ignores `n_periods` for DDD because `_ddd_dgp_kwargs` is hard-coded to the cross-sectional `generate_ddd_data`. Now that `generate_ddd_panel_data` exists (Wave 4), add a new `_EstimatorProfile` registry entry (or extend the existing one) to route to the panel DGP when `n_periods > 2`. | `power.py`, `prep_dgp.py` | follow-up | Low |
9796
| StaggeredTripleDifference R cross-validation: CSV fixtures not committed (gitignored); tests skip without local R + triplediff. Commit fixtures or generate deterministically. | `tests/test_methodology_staggered_triple_diff.py` | #245 | Medium |
9897
| StaggeredTripleDifference R parity: benchmark only tests no-covariate path (xformla=~1). Add covariate-adjusted scenarios and aggregation SE parity assertions. | `benchmarks/R/benchmark_staggered_triplediff.R` | #245 | Medium |
9998
| StaggeredTripleDifference: per-cohort group-effect SEs include WIF (conservative vs R's wif=NULL). Documented in REGISTRY. Could override mixin for exact R match. | `staggered_triple_diff.py` | #245 | Low |

diff_diff/power.py

Lines changed: 214 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,29 @@ def _ddd_dgp_kwargs(
267267
)
268268

269269

270+
def _ddd_panel_dgp_kwargs(
271+
n_units: int,
272+
n_periods: int,
273+
treatment_effect: float,
274+
treatment_fraction: float,
275+
treatment_period: int,
276+
sigma: float,
277+
) -> Dict[str, Any]:
278+
# Panel DDD DGP (n_periods > 2). `n_units` maps directly (no 8-cell //8
279+
# rounding). `treatment_fraction` is intentionally NOT mapped — DDD is a
280+
# balanced factorial, so group_frac/partition_frac are left at the DGP
281+
# default 0.5. Omitting group_frac/partition_frac here also keeps them out
282+
# of the _PROTECTED_DGP_KEYS collision check, so a user can still override
283+
# the group/partition split via data_generator_kwargs.
284+
return dict(
285+
n_units=n_units,
286+
n_periods=n_periods,
287+
treatment_period=treatment_period,
288+
treatment_effect=treatment_effect,
289+
noise_sd=sigma,
290+
)
291+
292+
270293
# -- Fit kwargs builders ------------------------------------------------------
271294

272295

@@ -320,6 +343,19 @@ def _ddd_fit_kwargs(
320343
return dict(outcome="outcome", group="group", partition="partition", time="time")
321344

322345

346+
def _ddd_panel_fit_kwargs(
347+
data: pd.DataFrame,
348+
n_units: int,
349+
n_periods: int,
350+
treatment_period: int,
351+
) -> Dict[str, Any]:
352+
# Panel DDD: time="post" is generate_ddd_panel_data's derived binary
353+
# pre/post indicator (vs the cross-sectional "time"). Clustering is NOT a
354+
# fit kwarg — it resolves from the estimator's cluster="unit" attribute
355+
# against the DGP's "unit" column.
356+
return dict(outcome="outcome", group="group", partition="partition", time="post")
357+
358+
323359
def _trop_fit_kwargs(
324360
data: pd.DataFrame,
325361
n_units: int,
@@ -636,6 +672,47 @@ def _ddd_effective_n(
636672
return eff if eff != n_units else None
637673

638674

675+
def _ddd_panel_cells_populated(n: int, group_frac: float, partition_frac: float) -> bool:
676+
"""Whether ``generate_ddd_panel_data`` would populate all 4 (group,partition)
677+
cells at ``n`` units. Mirrors the rounded stratified allocation + non-empty
678+
validation in ``prep_dgp.generate_ddd_panel_data`` (kept in lockstep)."""
679+
if n < 4:
680+
return False
681+
n_g1 = int(round(n * group_frac))
682+
n_g0 = n - n_g1
683+
n_p1_g0 = int(round(n_g0 * partition_frac))
684+
n_p1_g1 = int(round(n_g1 * partition_frac))
685+
cells = (n_g0 - n_p1_g0, n_p1_g0, n_g1 - n_p1_g1, n_p1_g1)
686+
return min(cells) >= 1
687+
688+
689+
def _ddd_panel_viable_min_n(
690+
group_frac: float, partition_frac: float, floor: int = 16, search_max: int = 100000
691+
) -> int:
692+
"""Smallest ``n_units`` for which ``generate_ddd_panel_data`` populates all
693+
four (group,partition) cells under the given split, floored at ``floor``.
694+
695+
For the balanced default (0.5/0.5) this is 4, so the result is ``floor``;
696+
skewed splits (e.g. 0.1/0.1) need more units before every cell is non-empty,
697+
so the sample-size search must bracket above this value (the registry
698+
documents group_frac/partition_frac as data_generator_kwargs overrides)."""
699+
# Validate up front (matching generate_ddd_panel_data) so an out-of-range
700+
# split raises the same clear message here — before the bracketing logic can
701+
# surface a misleading "n_range below the minimum" error downstream.
702+
if not (0.0 < group_frac < 1.0):
703+
raise ValueError(f"group_frac must be in (0, 1); got {group_frac}.")
704+
if not (0.0 < partition_frac < 1.0):
705+
raise ValueError(f"partition_frac must be in (0, 1); got {partition_frac}.")
706+
for n in range(4, search_max + 1):
707+
if _ddd_panel_cells_populated(n, group_frac, partition_frac):
708+
return max(floor, n)
709+
raise ValueError(
710+
f"No panel-DDD sample size <= {search_max} populates all four "
711+
f"(group, partition) cells for group_frac={group_frac}, "
712+
f"partition_frac={partition_frac}; move the split closer to 0.5."
713+
)
714+
715+
639716
def _check_ddd_dgp_compat(
640717
n_units: int,
641718
n_periods: int,
@@ -686,6 +763,55 @@ def _check_ddd_dgp_compat(
686763
)
687764

688765

766+
def _check_ddd_panel_dgp_compat(
767+
estimator: Any,
768+
treatment_fraction: float,
769+
data_generator_kwargs: Optional[Dict[str, Any]],
770+
) -> None:
771+
"""Compat checks for the panel DDD power path (``n_periods > 2``).
772+
773+
Unlike the cross-sectional ``_check_ddd_dgp_compat``, ``n_periods`` and
774+
``treatment_period`` are honored here (no warning). ``treatment_fraction``
775+
is still inert (the panel DGP is a balanced 2×2×2). The key addition is the
776+
clustering caveat: ``generate_ddd_panel_data`` has within-unit serial
777+
correlation, so unclustered SEs overstate power.
778+
"""
779+
overrides = data_generator_kwargs or {}
780+
if "n_per_cell" in overrides:
781+
raise ValueError(
782+
"data_generator_kwargs contains 'n_per_cell', a cross-sectional "
783+
"generate_ddd_data parameter. The panel DDD power path "
784+
"(n_periods > 2) uses generate_ddd_panel_data, which sizes the panel "
785+
"by n_units directly. Control the design via n_units and the "
786+
"group_frac / partition_frac data_generator_kwargs instead."
787+
)
788+
789+
if treatment_fraction != 0.5:
790+
warnings.warn(
791+
f"treatment_fraction={treatment_fraction} is ignored for "
792+
f"TripleDifference power: generate_ddd_panel_data uses a balanced "
793+
f"2×2×2 design (group_frac=partition_frac=0.5). Pass group_frac / "
794+
f"partition_frac via data_generator_kwargs to vary the split.",
795+
UserWarning,
796+
stacklevel=2,
797+
)
798+
799+
# The panel DGP hard-names its unit column "unit", and TripleDifference
800+
# resolves clustering from `self.cluster` against a same-named data column,
801+
# so on this auto-DGP path the only correct value is the literal "unit" —
802+
# this is NOT a general clustering check.
803+
if getattr(estimator, "cluster", None) != "unit":
804+
warnings.warn(
805+
"TripleDifference power on the panel DGP (n_periods > 2) has "
806+
"within-unit serial correlation, so unclustered standard errors are "
807+
"anti-conservative and overstate power. Construct the estimator as "
808+
'TripleDifference(cluster="unit") so the reported power reflects '
809+
"cluster-robust (Liang-Zeger CR1) inference.",
810+
UserWarning,
811+
stacklevel=2,
812+
)
813+
814+
689815
def _check_sdid_placebo_data(
690816
data: pd.DataFrame,
691817
estimator: Any,
@@ -833,6 +959,28 @@ def _get_registry() -> Dict[str, _EstimatorProfile]:
833959
return _ESTIMATOR_REGISTRY
834960

835961

962+
def _ddd_panel_profile() -> "_EstimatorProfile":
963+
"""Profile for panel DDD power simulations (n_periods > 2).
964+
965+
Routes TripleDifference power analysis to ``generate_ddd_panel_data``
966+
(which honors ``n_periods``/``treatment_period``) instead of the
967+
cross-sectional 2x2x2 ``generate_ddd_data``. See ``simulate_power``.
968+
"""
969+
from diff_diff.prep import generate_ddd_panel_data
970+
971+
# min_n=16 is intentionally lower than the cross-sectional DDD min_n=64
972+
# (which counts 8 cells x n_per_cell); the panel DGP maps n_units directly
973+
# and only requires n_units >= 4, so 16 gives >=1 unit per (group,partition)
974+
# cell with margin.
975+
return _EstimatorProfile(
976+
default_dgp=generate_ddd_panel_data,
977+
dgp_kwargs_builder=_ddd_panel_dgp_kwargs,
978+
fit_kwargs_builder=_ddd_panel_fit_kwargs,
979+
result_extractor=_extract_simple,
980+
min_n=16,
981+
)
982+
983+
836984
@dataclass
837985
class PowerResults:
838986
"""
@@ -2007,6 +2155,21 @@ def simulate_power(
20072155
use_custom_dgp = data_generator is not None
20082156
use_survey_dgp = survey_config is not None
20092157

2158+
# Route DDD power to the panel DGP when n_periods > 2. The cross-sectional
2159+
# 2x2x2 generate_ddd_data ignores n_periods; generate_ddd_panel_data honors
2160+
# it. Swapping the profile here (before the collision check and the DDD
2161+
# compat warnings) makes every downstream consumer (dgp_kwargs_builder,
2162+
# default_dgp, fit_kwargs_builder, result_extractor, min_n) use the panel
2163+
# variant automatically.
2164+
use_ddd_panel = (
2165+
estimator_name == "TripleDifference"
2166+
and n_periods > 2
2167+
and not use_custom_dgp
2168+
and not use_survey_dgp
2169+
)
2170+
if use_ddd_panel:
2171+
profile = _ddd_panel_profile()
2172+
20102173
# --- Survey config validation ---
20112174
if use_survey_dgp:
20122175
assert survey_config is not None # for type narrowing
@@ -2161,7 +2324,13 @@ def simulate_power(
21612324
)
21622325

21632326
# Warn if DDD design inputs are silently ignored
2164-
if estimator_name == "TripleDifference" and not use_custom_dgp:
2327+
if use_ddd_panel:
2328+
# Panel path honors n_periods/treatment_period; n_units maps directly
2329+
# (no //8 rounding). Different compat surface than the cross-sectional
2330+
# 2x2x2 design.
2331+
_check_ddd_panel_dgp_compat(estimator, treatment_fraction, data_generator_kwargs)
2332+
effective_n_units = None
2333+
elif estimator_name == "TripleDifference" and not use_custom_dgp:
21652334
_check_ddd_dgp_compat(
21662335
n_units,
21672336
n_periods,
@@ -2707,8 +2876,10 @@ def simulate_mde(
27072876
estimator_name = type(estimator).__name__
27082877
search_path: List[Dict[str, float]] = []
27092878

2710-
# Compute effective N for DDD (N is fixed throughout MDE search)
2711-
if estimator_name == "TripleDifference" and data_generator is None:
2879+
# Compute effective N for DDD (N is fixed throughout MDE search). Only the
2880+
# cross-sectional 2x2x2 DGP (n_periods <= 2) rounds n_units to a multiple of
2881+
# 8; the panel DGP (n_periods > 2) maps n_units directly, so report None.
2882+
if estimator_name == "TripleDifference" and data_generator is None and n_periods <= 2:
27122883
effective_n_units = _ddd_effective_n(n_units, data_generator_kwargs)
27132884
else:
27142885
effective_n_units = None
@@ -2925,15 +3096,33 @@ def simulate_sample_size(
29253096
estimator_name = type(estimator).__name__
29263097
search_path: List[Dict[str, float]] = []
29273098

2928-
# Determine min_n from registry
3099+
# Determine min_n from registry. DDD splits cross-sectional (n_periods <= 2,
3100+
# 2x2x2 factorial) from panel (n_periods > 2, generate_ddd_panel_data).
29293101
registry = _get_registry()
29303102
profile = registry.get(estimator_name)
2931-
min_n = profile.min_n if profile is not None else 20
3103+
is_ddd = estimator_name == "TripleDifference" and data_generator is None
3104+
is_ddd_panel = is_ddd and n_periods > 2
3105+
if is_ddd_panel:
3106+
# The panel DGP requires every (group,partition) cell non-empty, which
3107+
# for a skewed group_frac/partition_frac override needs more than the
3108+
# default 16-unit floor. Bracket above the viable minimum so the search
3109+
# never probes an infeasible n (which would raise in the DGP).
3110+
_ddd_overrides = data_generator_kwargs or {}
3111+
min_n = _ddd_panel_viable_min_n(
3112+
_ddd_overrides.get("group_frac", 0.5),
3113+
_ddd_overrides.get("partition_frac", 0.5),
3114+
floor=_ddd_panel_profile().min_n,
3115+
)
3116+
else:
3117+
min_n = profile.min_n if profile is not None else 20
29323118

2933-
# DDD grid snapping: bisection candidates must be multiples of 8
2934-
is_ddd_grid = estimator_name == "TripleDifference" and data_generator is None
3119+
# Grid snapping: the cross-sectional 2x2x2 DDD DGP rounds n_units to a
3120+
# multiple of 8, so bisection candidates must snap to that grid. The panel
3121+
# DGP maps n_units directly → continuous (step-1) search like every other
3122+
# estimator.
3123+
is_ddd_grid = is_ddd and not is_ddd_panel
29353124
grid_step = 8 if is_ddd_grid else 1
2936-
convergence_threshold = grid_step + 1 # 9 for DDD, 2 for others
3125+
convergence_threshold = grid_step + 1 # 9 for cross-sectional DDD, 2 otherwise
29373126

29383127
if is_ddd_grid and data_generator_kwargs and "n_per_cell" in data_generator_kwargs:
29393128
raise ValueError(
@@ -2991,9 +3180,25 @@ def _power_at_n(n: int) -> float:
29913180
)
29923181

29933182
# --- Bracket ---
2994-
abs_min = 16 if is_ddd_grid else 4
3183+
# Cross-sectional DDD wants a >=16 floor so the 8 G×P×T cells are populated;
3184+
# the panel path uses its split-aware viable floor (min_n above, which is
3185+
# >=16 and higher for skewed group_frac/partition_frac); everything else
3186+
# floors at 4.
3187+
if is_ddd_panel:
3188+
abs_min = min_n
3189+
elif is_ddd:
3190+
abs_min = 16
3191+
else:
3192+
abs_min = 4
29953193
if survey_config is not None:
29963194
abs_min = max(abs_min, survey_config.min_viable_n)
3195+
if is_ddd_panel and n_range is not None and n_range[1] < abs_min:
3196+
raise ValueError(
3197+
f"n_range upper bound ({n_range[1]}) is below the minimum panel-DDD "
3198+
f"sample size ({abs_min}) needed to populate all (group, partition) "
3199+
f"cells for group_frac/partition_frac. Raise the upper bound or move "
3200+
f"the split closer to 0.5."
3201+
)
29973202
if n_range is not None:
29983203
lo, hi = _snap_n(n_range[0], "up", floor=abs_min), _snap_n(
29993204
n_range[1], "down", floor=abs_min

0 commit comments

Comments
 (0)