Skip to content

Commit ba0eee6

Browse files
igerberclaude
andcommitted
Address PR #95 follow-up: fix bootstrap p-value floor and document policy
- Fix bootstrap p-value floor to use valid sample count (not n_bootstrap) - Remove unused _mask_nonfinite_samples method (dead code) - Document non-finite inference handling in Methodology Registry - Fix misleading test docstring that claimed undocumented behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f3c9f05 commit ba0eee6

3 files changed

Lines changed: 17 additions & 32 deletions

File tree

diff_diff/staggered_bootstrap.py

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -688,36 +688,17 @@ def _compute_effect_bootstrap_stats(
688688

689689
# Use only valid samples
690690
valid_dist = boot_dist[finite_mask]
691+
n_valid_bootstrap = len(valid_dist)
691692

692693
se = float(np.std(valid_dist, ddof=1))
693694
ci = self._compute_percentile_ci(valid_dist, self.alpha)
694-
p_value = self._compute_bootstrap_pvalue(original_effect, valid_dist)
695-
return se, ci, p_value
696695

697-
def _mask_nonfinite_samples(self, arr: np.ndarray, context: str) -> np.ndarray:
698-
"""Return boolean mask of finite samples, warning if any dropped.
699-
700-
Parameters
701-
----------
702-
arr : np.ndarray
703-
Array to check (1D bootstrap distribution).
704-
context : str
705-
Description of where this check is happening (for warning message).
696+
# Compute p-value inline with correct floor based on valid sample count
697+
if original_effect >= 0:
698+
p_one_sided = np.mean(valid_dist <= 0)
699+
else:
700+
p_one_sided = np.mean(valid_dist >= 0)
701+
p_value = min(2 * p_one_sided, 1.0)
702+
p_value = max(p_value, 1 / (n_valid_bootstrap + 1)) # Floor uses valid count
706703

707-
Returns
708-
-------
709-
np.ndarray
710-
Boolean mask where True indicates finite (valid) samples.
711-
"""
712-
finite_mask = np.isfinite(arr)
713-
if not np.all(finite_mask):
714-
import warnings
715-
n_nonfinite = np.sum(~finite_mask)
716-
warnings.warn(
717-
f"Dropping {n_nonfinite}/{arr.size} non-finite bootstrap samples in {context}. "
718-
"This may occur with very small samples or extreme weights. "
719-
"Bootstrap estimates based on remaining valid samples.",
720-
RuntimeWarning,
721-
stacklevel=3
722-
)
723-
return finite_mask
704+
return se, ci, float(p_value)

docs/methodology/REGISTRY.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,10 @@ Aggregations:
204204
- Detection: Pivoted QR decomposition with tolerance `1e-07` (R's `qr()` default)
205205
- Handling: Warns and drops linearly dependent columns, sets NA for dropped coefficients (R-style, matches `lm()`)
206206
- Parameter: `rank_deficient_action` controls behavior: "warn" (default), "error", or "silent"
207+
- Non-finite inference values:
208+
- Analytic SE: Returns NaN to signal invalid inference (not biased via zeroing)
209+
- Bootstrap: Drops non-finite samples, warns, and adjusts p-value floor accordingly
210+
- Threshold: Returns NaN if <50% of bootstrap samples are valid
207211

208212
**Reference implementation(s):**
209213
- R: `did::att_gt()` (Callaway & Sant'Anna's official package)

tests/test_staggered.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -684,10 +684,10 @@ def test_extreme_propensity_scores(self):
684684
def test_extreme_weights_warning(self):
685685
"""Test that extreme weights produce warnings and methodology-aligned behavior.
686686
687-
Per the Methodology Registry (docs/methodology/REGISTRY.md):
688-
- Missing group-time cells: ATT(g,t) set to NaN
689-
- Analytic SE: returns NaN to signal invalid inference (not biased via zeroing)
690-
- Bootstrap: drops invalid samples and warns, preserving valid distribution
687+
Tests that:
688+
- ATT point estimates remain finite
689+
- SE is finite (valid) or NaN (signals invalid inference), never biased
690+
- Bootstrap drops invalid samples and adjusts inference accordingly
691691
"""
692692
import warnings
693693
np.random.seed(42)

0 commit comments

Comments
 (0)