Skip to content

Commit 685c926

Browse files
igerberclaude
andcommitted
fix: address CI review round 2 - block heterogeneous TE, fix hi-bracket, docstrings
- P0: reject heterogeneous_te_by_strata=True in survey power validation (DGP population_att diverges from input treatment_effect, making bias/coverage/RMSE metrics misleading) - P1: fix simulate_sample_size auto-bracketing hi to respect abs_min (hi = max(2*lo, abs_min, 100) prevents probing below survey floor) - P2: add survey_config parameter to simulate_power/mde/sample_size docstrings with constraints and supported estimators - P2: add regression tests for heterogeneous_te rejection and large-floor (n_strata=10, psu_per_stratum=10, min_viable_n=200) auto-bracketing branch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 40acf5b commit 685c926

2 files changed

Lines changed: 53 additions & 1 deletion

File tree

diff_diff/power.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1812,6 +1812,13 @@ def simulate_power(
18121812
estimators with non-standard result schemas.
18131813
progress : bool, default=True
18141814
Whether to print progress updates.
1815+
survey_config : SurveyPowerConfig, optional
1816+
When provided, generates survey-structured data via
1817+
``generate_survey_did_data`` and injects ``SurveyDesign`` into
1818+
estimator ``fit()``. Mutually exclusive with ``data_generator``.
1819+
Supported estimators: DiD, TWFE, MultiPeriod, CS, SA, Imputation,
1820+
TwoStage, Stacked, Efficient. Unsupported: TROP, SyntheticDiD,
1821+
TripleDifference. ``heterogeneous_te_by_strata`` must be False.
18151822
18161823
Returns
18171824
-------
@@ -1906,6 +1913,13 @@ def simulate_power(
19061913
f"No survey power profile for {estimator_name}. "
19071914
f"Supported: {sorted(_SURVEY_FIT_BUILDERS.keys())}."
19081915
)
1916+
if survey_config.heterogeneous_te_by_strata:
1917+
raise ValueError(
1918+
"heterogeneous_te_by_strata=True is not supported with "
1919+
"simulation power analysis. The DGP's population ATT diverges "
1920+
"from the input treatment_effect under heterogeneous effects, "
1921+
"which would make bias/coverage/RMSE metrics misleading."
1922+
)
19091923

19101924
data_gen_kwargs = data_generator_kwargs or {}
19111925
est_kwargs = estimator_kwargs or {}
@@ -2482,6 +2496,9 @@ def simulate_mde(
24822496
Forwarded to ``simulate_power()``.
24832497
progress : bool, default=True
24842498
Whether to print progress updates.
2499+
survey_config : SurveyPowerConfig, optional
2500+
Survey-aware simulation config. Forwarded to ``simulate_power()``.
2501+
See :func:`simulate_power` for details and constraints.
24852502
24862503
Returns
24872504
-------
@@ -2693,6 +2710,11 @@ def simulate_sample_size(
26932710
Forwarded to ``simulate_power()``.
26942711
progress : bool, default=True
26952712
Whether to print progress updates.
2713+
survey_config : SurveyPowerConfig, optional
2714+
Survey-aware simulation config. Forwarded to ``simulate_power()``.
2715+
When set, the bisection floor is raised to
2716+
``survey_config.min_viable_n`` to ensure viable survey structure.
2717+
See :func:`simulate_power` for details and constraints.
26962718
26972719
Returns
26982720
-------
@@ -2847,7 +2869,7 @@ def _power_at_n(n: int) -> float:
28472869
)
28482870
# Fall through to bisection with lo..hi bracket
28492871
else:
2850-
hi = max(100, 2 * min_n)
2872+
hi = max(2 * lo, abs_min, 100)
28512873
for _ in range(10):
28522874
if _power_at_n(hi) >= power:
28532875
break

tests/test_power.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2377,6 +2377,36 @@ def test_survey_sample_size_large_effect_floor(self):
23772377
)
23782378
assert result.required_n >= cfg.min_viable_n # must be >= 80
23792379

2380+
def test_survey_sample_size_large_floor_auto_bracket(self):
2381+
"""Auto-bracketing hi respects min_viable_n > 100."""
2382+
cfg = SurveyPowerConfig(n_strata=10, psu_per_stratum=10)
2383+
# min_viable_n = 10 * 10 * 2 = 200, which exceeds default hi=100
2384+
result = simulate_sample_size(
2385+
CallawaySantAnna(),
2386+
treatment_effect=0.5,
2387+
sigma=3.0,
2388+
n_simulations=10,
2389+
max_steps=3,
2390+
seed=42,
2391+
survey_config=cfg,
2392+
n_periods=4,
2393+
treatment_period=2,
2394+
progress=False,
2395+
)
2396+
assert result.required_n >= cfg.min_viable_n # must be >= 200
2397+
2398+
def test_survey_rejects_heterogeneous_te(self):
2399+
"""heterogeneous_te_by_strata=True rejected with simulation power."""
2400+
cfg = SurveyPowerConfig(heterogeneous_te_by_strata=True)
2401+
with pytest.raises(ValueError, match="heterogeneous_te_by_strata"):
2402+
simulate_power(
2403+
CallawaySantAnna(),
2404+
n_simulations=1,
2405+
seed=42,
2406+
survey_config=cfg,
2407+
**_SIM_KW,
2408+
)
2409+
23802410
# -- Closed-form deff tests --
23812411

23822412
def test_closed_form_deff_default(self):

0 commit comments

Comments
 (0)