Skip to content

Commit a1a522d

Browse files
igerberclaude
andcommitted
Address CI review: default n_simulation_failures=0 + all-failed test
Two items from CI AI review on PR #326: 1. P2 backward-compat: moving `n_simulation_failures` to the end of `SimulationPowerResults` with a default of `0`. Users who manually instantiate the dataclass with the pre-PR field order continue to work; `simulate_power()` still fills the field in via keyword. The field remains part of `to_dict()` output (PR-level contract unchanged). 2. P3 coverage: adding a regression test for the all-failed escape path. An estimator that raises `ValueError` on every replicate now asserts both the `RuntimeError("All simulations failed. ...")` message and that the narrow-except filter doesn't swallow it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent af56950 commit a1a522d

2 files changed

Lines changed: 19 additions & 1 deletion

File tree

diff_diff/power.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,6 @@ class SimulationPowerResults:
10261026
mean_se: float
10271027
coverage: float
10281028
n_simulations: int
1029-
n_simulation_failures: int
10301029
effect_sizes: List[float]
10311030
powers: List[float]
10321031
true_effect: float
@@ -1039,6 +1038,7 @@ class SimulationPowerResults:
10391038
survey_config: Optional[Any] = field(default=None, repr=False)
10401039
mean_deff: Optional[float] = None
10411040
mean_icc_realized: Optional[float] = None
1041+
n_simulation_failures: int = 0
10421042

10431043
def __post_init__(self):
10441044
"""Compute derived statistics."""

tests/test_power.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,24 @@ def fit(self, data, **kwargs):
626626
data_generator=generate_did_data,
627627
)
628628

629+
def test_simulation_all_failed_raises_runtime_error(self):
630+
"""All simulations failing: narrow-except path still raises RuntimeError."""
631+
from diff_diff.prep import generate_did_data
632+
633+
class AlwaysFailingEstimator:
634+
def fit(self, data, **kwargs):
635+
raise ValueError("every replicate fails")
636+
637+
with warnings.catch_warnings():
638+
warnings.simplefilter("ignore", UserWarning)
639+
with pytest.raises(RuntimeError, match="All simulations failed"):
640+
simulate_power(
641+
estimator=AlwaysFailingEstimator(),
642+
n_simulations=5,
643+
progress=False,
644+
data_generator=generate_did_data,
645+
)
646+
629647
def test_simulation_failure_counter_survives_serialization(self):
630648
"""`n_simulation_failures` round-trips through to_dict/to_dataframe."""
631649
from diff_diff.prep import generate_did_data

0 commit comments

Comments
 (0)