perf(utils): precompute r-independent wild-bootstrap inversion pieces (~7x, outputs unchanged)#647
perf(utils): precompute r-independent wild-bootstrap inversion pieces (~7x, outputs unchanged)#647igerber wants to merge 2 commits into
Conversation
|
Overall Assessment ✅ Looks good. I found no unmitigated P0/P1 issues. Executive Summary
Methodology No findings. Cross-check: Code Quality No findings. The chunk sizing and precompute loop are localized and keep the existing result surface unchanged: Performance No findings. The change removes repeated Maintainability No findings. The explanatory comments are long but useful here because this is math-sensitive code. Tech Debt
Security No findings. Documentation/Tests
Test coverage added for chunk-count invariance at |
d382c4e to
8027a33
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment ✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
Methodology No findings. Cross-check: Code Quality No findings. The chunk-budget constant and chunk loop are localized at Performance No findings. The PR removes repeated observation-level Maintainability No findings. The explanatory comments are long, but appropriate for math-sensitive inference code. Tech Debt
Security No findings. Documentation/Tests
|
…gregation The cluster-score aggregation in compute_robust_vcov (also reached via solve_ols(return_vcov=True)) built its (G,k) cluster-scores matrix in HashMap iteration order, which is SipHash-randomized per call: mathematically identical, but the GEMM accumulation order changed on every invocation, wobbling the clustered vcov at ~1e-14 (3 distinct values across 8 identical calls; the Python backend is bit-stable). Rows now accumulate in first-appearance order — ascending for the factorized 0..G-1 ids the Python dispatcher passes, matching NumPy's groupby order. Verified 1 distinct value across 50 identical calls; NumPy parity unchanged (~1e-14 cross-backend GEMM tolerance). Bit-identity regression tests cover contiguous and non-contiguous unsorted cluster ids. Resolves the TODO row currently in flight on PR #647 (row deletion follows once both merge). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
8027a33 to
5b3f686
Compare
…st inversion (~7x, bit-identical outputs) The WCR bootstrap DGP is linear in the candidate null r (y*(r) = A + r*B), so beta_j*(r) is affine and the per-cluster score variance is the PSD quadratic qa + r*qb + r^2*qc — five precomputable (B,) vectors. The CI inversion's ~O(100) _t_star(r) evaluations (each previously materializing fresh (B,n)/(k,B)/(n,B) arrays) become O(B) arithmetic; the one-time precompute pass is chunked over draws (~256MB cap) so peak memory is bounded for large n/B — resolving both options the TODO row offered at once. Verified vs origin/main on a 5-seed few-cluster grid (backend pinned): SE, p-value, and inverted CI endpoints all bit-identical; 6.8x end-to-end. boottest parity suite (60 tests, pinned values) unchanged. Also parks the CS RC pscore (g,t)-dedup TODO row with findings: the RC propensity is fit on the pooled two-period sample whose period-t blocks change with every t, so no two cells share a logit — caching would change the estimator away from DRDID::*_rc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5b3f686 to
e7a866b
Compare
…gregation The cluster-score aggregation in compute_robust_vcov (also reached via solve_ols(return_vcov=True)) built its (G,k) cluster-scores matrix in HashMap iteration order, which is SipHash-randomized per call: mathematically identical, but the GEMM accumulation order changed on every invocation, wobbling the clustered vcov at ~1e-14 (3 distinct values across 8 identical calls; the Python backend is bit-stable). Rows now accumulate in first-appearance order — ascending for the factorized 0..G-1 ids the Python dispatcher passes, matching NumPy's groupby order. Verified 1 distinct value across 50 identical calls; NumPy parity unchanged (~1e-14 cross-backend GEMM tolerance). Bit-identity regression tests cover contiguous and non-contiguous unsorted cluster ids. Resolves the TODO row currently in flight on PR #647 (row deletion follows once both merge). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment ✅ Looks good. No unmitigated P0/P1 findings. Executive Summary
Methodology No findings. Cross-check: Code Quality No findings. The chunk-budget constant and chunked precompute are localized at Performance No findings. The change removes repeated Maintainability No findings. The explanatory comments are long, but appropriate for math-sensitive inference code. Tech Debt
Security No findings. Documentation/Tests No findings. Prior P3s are addressed: |
Summary
r(y*(r) = A + r·Bwith r-independentA,B), sobeta_j*(r)is affine and the per-cluster score variance is the PSD quadraticqa + r·qb + r²·qc— five precomputable(B,)vectors. The CI inversion's ~O(100)_t_star(r)evaluations, which each materialized fresh(B×n)bootstrap-outcome,(k×B)refit, and(n×B)residual arrays, now cost O(B) arithmetic each. This lands BOTH options the TODO row offered: the one-time precompute pass is chunked over draws (module byte-budget constant_WILD_PRECOMPUTE_CHUNK_BYTES, sized with a conservative ×8 live-temporary multiplier and monkeypatchable), so peak memory is bounded for largen/B, and no full(B, n)observation-level weight matrix is ever materialized.boottestparity suite (61 tests incl. pinned values) passes unchanged. The quadratic-form evaluation is a ~1-ULP reassociation of the per-callsum(scores²); the strict-inequality tie guard absorbs sub-1e-9 shifts by design. A newTestPrecomputeChunkingforces hundreds of draw-chunks and pins chunk-count invariance.solve_ols(return_vcov=True)is run-to-run nondeterministic (~1e-14 rel; 3 distinct clustered-vcov values in 8 identical calls on Apple Silicon; pure-Python backend is bit-stable). Not introduced by this PR (it studentizes with the pre-existing analytical CR1) — tracked for investigation with a repro recipe.tblocks change with everyt, so no two cells share a logit — caching would change the estimator away fromDRDID::*_rc.Methodology references (required if estimator / math changes)
ris the documented basis of the inversionValidation
tests/test_wild_bootstrap.py::TestPrecomputeChunking(chunk-count invariance; tolerances documented against the ambient rust-vcov wobble). Full wild suite 61 passed;tests/test_utils.py,tests/test_methodology_did.py,tests/test_estimators.pypass.Security / privacy
Generated with Claude Code