Skip to content

Commit d5d0ac0

Browse files
igerberclaude
andcommitted
test+docs: chunk-count invariance is ~1 ULP, not bit-for-bit (CI failure fix)
CI (linux-arm + Windows) failed test_contrast_chunking_bit_identical at 1-ULP differences: each contrast's B is computed independently, but the per-cluster GEMM X_g' omega_g[:, c0:c1] runs over a width-c slice and BLAS kernels (GEMV vs GEMM, platform-dependent) may accumulate a column differently at width 1 vs width m — exact on Accelerate, 1-ULP drift elsewhere. The documented chunking-reassociation caveat applies: test renamed to test_contrast_chunking_invariant with assert_allclose(rtol=1e-13), and the bit-for-bit claims corrected in the module-constant comment, docstring, CHANGELOG, and REGISTRY. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 576d070 commit d5d0ac0

4 files changed

Lines changed: 22 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
562562
contrast, `O(n²)` memory — 3.2 GB at n=20k). The pairwise matrix now collapses to
563563
`B = diag(‖ω_g‖²) − P'M_U P` with `P = X'Ω` (disjoint cluster supports), costing
564564
`O(nk + G²k)` per contrast with no `n×n` allocation (per-cluster product buffers
565-
contrast-chunked under a 64 MB cap, bit-identical across chunk counts, so the batched
565+
contrast-chunked under a 64 MB cap, chunk-count invariant to ~1 ULP — BLAS kernels
566+
can accumulate a GEMM column differently at different slice widths — so the batched
566567
per-coefficient sweep never allocates `O(G·k²)` at once): ~32x at n=5k/G=50 (0.57s→0.018s);
567568
n=20k/G=100 completes in 0.12s where the old path would allocate 3.2 GB. Algebraically
568569
identical — agreement with a frozen pair-loop oracle at rtol 1e-10 (balanced,

diff_diff/linalg.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2153,8 +2153,12 @@ def _compute_cr2_bm(
21532153
# (G, k, chunk) per-cluster product buffer so a batched per-coefficient sweep
21542154
# (contrasts=eye(k), i.e. m == k) cannot allocate O(G*k*m) at once on
21552155
# full-dummy / absorbed-FE designs with many clusters and coefficients.
2156-
# Results are chunk-count invariant BIT-FOR-BIT: each contrast's B matrix is
2157-
# computed independently, so chunking never reassociates a contrast's sums.
2156+
# Each contrast's B matrix is computed independently, so chunking never
2157+
# reassociates a contrast's OWN sums — but the per-cluster GEMM
2158+
# `X_g' omega_g[:, c0:c1]` runs over a width-c slice, and BLAS kernels
2159+
# (GEMV vs GEMM, platform-dependent) may accumulate a column differently at
2160+
# different widths: chunk-count invariance holds to ~1 ULP (observed exact
2161+
# on Accelerate, 1-ULP drift on OpenBLAS/arm + Windows), NOT bit-for-bit.
21582162
# Module-level so tests can monkeypatch it to force the multi-chunk path.
21592163
_CR2_BM_CONTRAST_CHUNK_BYTES = 64 * 1024 * 1024
21602164

@@ -2197,7 +2201,9 @@ def _cr2_bm_dof_inner(
21972201
21982202
so ``trace_B2 = ||B||_F^2`` costs ``O(n k + G^2 k)`` per contrast with
21992203
``O(G k)`` memory per contrast (contrast-chunked buffers bounded by
2200-
``_CR2_BM_CONTRAST_CHUNK_BYTES``, bit-identical across chunk counts) — the previous form materialized the dense ``n x n``
2204+
``_CR2_BM_CONTRAST_CHUNK_BYTES``, chunk-count invariant to ~1 ULP — BLAS
2205+
kernels may accumulate a GEMM column differently at different slice
2206+
widths) — the previous form materialized the dense ``n x n``
22012207
``M`` and looped cluster pairs at ``O(n^2)`` per contrast, the exact
22022208
large-``n`` blowup the TODO row tracked. The two evaluations are
22032209
algebraically identical; floating-point agreement is ~1e-12 relative
@@ -2233,7 +2239,8 @@ def _cr2_bm_dof_inner(
22332239
# Chunk the contrasts so the per-cluster product buffer is (G, k, c) with
22342240
# c bounded by _CR2_BM_CONTRAST_CHUNK_BYTES — a full-m buffer would be
22352241
# O(G*k*m), i.e. O(G*k^2) on the batched per-coefficient sweep. Each
2236-
# contrast's B is computed independently, so chunking is bit-identical.
2242+
# contrast's B is computed independently; chunk-count invariance holds
2243+
# to ~1 ULP (BLAS width-dependent column accumulation), not bit-for-bit.
22372244
chunk = max(1, int(_CR2_BM_CONTRAST_CHUNK_BYTES // max(n_g_clusters * k_X * 8, 1)))
22382245
for c0 in range(0, m, chunk):
22392246
c1 = min(c0 + chunk, m)

docs/methodology/REGISTRY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ where V is the VCV sub-matrix for post-treatment δ_e coefficients.
243243
`_compute_cr2_bm_contrast_dof` (DOF-only for arbitrary contrasts) are thin wrappers
244244
over that shared core, so every CR2 caller routes through one implementation. The
245245
consolidation is bit-identical to the prior two-call path (proven at atol=0).
246-
- **Note (unweighted per-coef DOF guard):** the unweighted clustered CR2-BM per-coefficient DOF (`_cr2_bm_dof_inner`, the simple `(tr B)²/tr(B²)` form) carries the same two-part reliability guard as the weighted P-array path. (1) **Noise floor:** for a high-leverage FE-dummy / collinear nuisance column `trace_B2 = Σ B_{g,h}²` collapses to float64 accumulation noise while `trace_B` stays O(1), inflating the ratio to a non-physical DOF (observed ~1e61 on the absorbed-FE golden); a contrast whose `max|B_{g,h}|` sits below the batch-relative (`1e-10×max`, computed on the scale-normalized `max|B|/‖c‖²` since `B ∝ ‖c‖²` while the DOF is scale-invariant) or absolute (`(EPS·n·k·bread_scale)²`) floor is NaN'd. (2) **Cluster-count bound:** the Bell-McCaffrey Satterthwaite DOF is `(tr B)²/tr(B²)` with `B` PSD and cluster-structured, so it is bounded by `rank(B) ≤ G` (number of clusters); the simple unweighted form is numerically less faithful than clubSandwich's P-array form on high-leverage columns and can return a finite-but-inflated DOF above `G` (observed ~32.7 and ~16.3 vs R's 6 and 3, `G=8`), which is NaN'd as non-physical. The well-conditioned contrasts estimators consume — the treatment effect, event-study coefficients, and the compound post-period-average — are unaffected and match R clubSandwich; only the non-user-facing high-leverage nuisance DOFs are suppressed (exact P-array reproduction of those is deferred). A `UserWarning` fires per fit. Regression: `tests/test_estimators_vcov_type.py::TestDiDAbsorbedFERParity::test_unweighted_cr2_bm_per_coef_dof_no_nonphysical`. **Evaluation (2026-07):** the pairwise `B` matrix is computed via the algebraic identity `B = Ω'MΩ = diag(‖ω_g‖²) − P' M_U P` with `P = X'Ω` (the Satterthwaite DOF itself is Pustejovsky-Tipton 2018 §3.1 / Eq. 13) — instead of contracting against an explicit dense `n×n` residual-maker (`O(n k + G² k)` per contrast, no `n×n` allocation, per-cluster product buffers contrast-chunked under a byte cap (bit-identical across chunk counts); algebraically identical, ~1e-12 float agreement locked by a frozen-oracle parity test).
246+
- **Note (unweighted per-coef DOF guard):** the unweighted clustered CR2-BM per-coefficient DOF (`_cr2_bm_dof_inner`, the simple `(tr B)²/tr(B²)` form) carries the same two-part reliability guard as the weighted P-array path. (1) **Noise floor:** for a high-leverage FE-dummy / collinear nuisance column `trace_B2 = Σ B_{g,h}²` collapses to float64 accumulation noise while `trace_B` stays O(1), inflating the ratio to a non-physical DOF (observed ~1e61 on the absorbed-FE golden); a contrast whose `max|B_{g,h}|` sits below the batch-relative (`1e-10×max`, computed on the scale-normalized `max|B|/‖c‖²` since `B ∝ ‖c‖²` while the DOF is scale-invariant) or absolute (`(EPS·n·k·bread_scale)²`) floor is NaN'd. (2) **Cluster-count bound:** the Bell-McCaffrey Satterthwaite DOF is `(tr B)²/tr(B²)` with `B` PSD and cluster-structured, so it is bounded by `rank(B) ≤ G` (number of clusters); the simple unweighted form is numerically less faithful than clubSandwich's P-array form on high-leverage columns and can return a finite-but-inflated DOF above `G` (observed ~32.7 and ~16.3 vs R's 6 and 3, `G=8`), which is NaN'd as non-physical. The well-conditioned contrasts estimators consume — the treatment effect, event-study coefficients, and the compound post-period-average — are unaffected and match R clubSandwich; only the non-user-facing high-leverage nuisance DOFs are suppressed (exact P-array reproduction of those is deferred). A `UserWarning` fires per fit. Regression: `tests/test_estimators_vcov_type.py::TestDiDAbsorbedFERParity::test_unweighted_cr2_bm_per_coef_dof_no_nonphysical`. **Evaluation (2026-07):** the pairwise `B` matrix is computed via the algebraic identity `B = Ω'MΩ = diag(‖ω_g‖²) − P' M_U P` with `P = X'Ω` (the Satterthwaite DOF itself is Pustejovsky-Tipton 2018 §3.1 / Eq. 13) — instead of contracting against an explicit dense `n×n` residual-maker (`O(n k + G² k)` per contrast, no `n×n` allocation, per-cluster product buffers contrast-chunked under a byte cap (chunk-count invariant to ~1 ULP — BLAS width-dependent column accumulation); algebraically identical, ~1e-12 float agreement locked by a frozen-oracle parity test).
247247
- **Note:** `LinearRegression.get_se()` / `get_inference()` clamp the vcov diagonal at 0
248248
before `sqrt`. A high-leverage / degenerate coefficient (an absorbed-FE dummy
249249
near-collinear with the treatment, whose Satterthwaite DOF already hits the noise-floor

tests/test_linalg.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2808,11 +2808,15 @@ def test_unbalanced_clusters_match_oracle(self):
28082808
oracle = self._oracle_pairloop_dof(X, cl, bread, np.eye(k))
28092809
np.testing.assert_allclose(dof, oracle, rtol=1e-10)
28102810

2811-
def test_contrast_chunking_bit_identical(self, monkeypatch):
2811+
def test_contrast_chunking_invariant(self, monkeypatch):
28122812
"""CI-review P2: the per-cluster product buffer is contrast-chunked
28132813
(bounded by _CR2_BM_CONTRAST_CHUNK_BYTES) instead of O(G*k*m).
2814-
Forcing one-contrast chunks must reproduce the single-chunk DOF
2815-
bit-for-bit — each contrast's B is computed independently."""
2814+
Forcing one-contrast chunks reproduces the single-chunk DOF to
2815+
~1 ULP: each contrast's B is computed independently, but the
2816+
per-cluster GEMM runs over a width-c slice and BLAS kernels may
2817+
accumulate a column differently at width 1 vs width m (observed
2818+
exact on Accelerate, 1-ULP drift on OpenBLAS/arm + Windows CI —
2819+
the documented chunking-reassociation caveat)."""
28162820
import diff_diff.linalg as la
28172821

28182822
rng = np.random.default_rng(23)
@@ -2826,4 +2830,4 @@ def test_contrast_chunking_bit_identical(self, monkeypatch):
28262830
_, dof_one = la._compute_cr2_bm(X, resid, cl, bread)
28272831
monkeypatch.setattr(la, "_CR2_BM_CONTRAST_CHUNK_BYTES", G * k * 8) # 1 contrast/chunk
28282832
_, dof_many = la._compute_cr2_bm(X, resid, cl, bread)
2829-
np.testing.assert_array_equal(dof_many, dof_one)
2833+
np.testing.assert_allclose(dof_many, dof_one, rtol=1e-13)

0 commit comments

Comments
 (0)