Skip to content

Commit 87661d7

Browse files
igerberclaude
andcommitted
fix: address CI review round 10 - _snap_n floor enforcement for n_range
- P1: fix _snap_n() to honor floor even when grid_step==1 (non-DDD), so explicit n_range with survey_config is clamped to min_viable_n - Add regression test for n_range=(10, 200) with min_viable_n=80 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b91a485 commit 87661d7

2 files changed

Lines changed: 21 additions & 2 deletions

File tree

diff_diff/power.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2834,9 +2834,9 @@ def simulate_sample_size(
28342834
)
28352835

28362836
def _snap_n(n: int, direction: str = "down", floor: Optional[int] = None) -> int:
2837-
if grid_step == 1:
2838-
return n
28392837
actual_floor = floor if floor is not None else min_n
2838+
if grid_step == 1:
2839+
return max(actual_floor, n)
28402840
if direction == "up":
28412841
return max(actual_floor, ((n + grid_step - 1) // grid_step) * grid_step)
28422842
return max(actual_floor, (n // grid_step) * grid_step)

tests/test_power.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2395,6 +2395,25 @@ def test_survey_sample_size_large_floor_auto_bracket(self):
23952395
)
23962396
assert result.required_n >= cfg.min_viable_n # must be >= 200
23972397

2398+
def test_survey_sample_size_explicit_n_range_clamped(self):
2399+
"""Explicit n_range below survey floor is clamped to min_viable_n."""
2400+
cfg = SurveyPowerConfig(n_strata=5, psu_per_stratum=8)
2401+
# n_range=(10, 200) but min_viable_n=80, so lo should be clamped to 80
2402+
result = simulate_sample_size(
2403+
CallawaySantAnna(),
2404+
treatment_effect=3.0,
2405+
sigma=1.0,
2406+
n_range=(10, 200),
2407+
n_simulations=10,
2408+
max_steps=3,
2409+
seed=42,
2410+
survey_config=cfg,
2411+
n_periods=4,
2412+
treatment_period=2,
2413+
progress=False,
2414+
)
2415+
assert result.required_n >= cfg.min_viable_n # must be >= 80
2416+
23982417
def test_survey_rejects_heterogeneous_te(self):
23992418
"""heterogeneous_te_by_strata=True rejected with simulation power."""
24002419
cfg = SurveyPowerConfig(heterogeneous_te_by_strata=True)

0 commit comments

Comments
 (0)