From b0d3f413813fb9391d0d2a4e73afc53b3184206f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 18:06:55 +0000 Subject: [PATCH] fix(test): degenerate-band sanity check asserted on BLAS rounding, not the library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test__fnnls_cholesky__never_returns_a_non_finite_solution[0.0]` fails on the CI runners while passing locally, and has been red on main since at least efaf3041 (run 31219374045). The failing line is the test's own vacuity guard: assert raised > 0 It requires `fnnls_cholesky` to raise LinAlgError for at least one of 40 seeds. But raising is an *allowed* outcome here, not a required one — this file already says so, in `test__cholinsertlast__singular_insertion_never_yields_an_unusable_pivot`: The invariant is therefore NOT "always raise" (a small positive pivot is still a usable pivot, and rejecting it would change likelihood evaluations). It is: either raise, or return a strictly positive finite pivot. On a singular insertion the Schur complement is zero up to rounding, so which side of zero it lands on is decided by floating-point summation order — the module comment says exactly this, and cites it as why the original bug reproduced on CI but not locally. `raised > 0` therefore asserts on the runner's BLAS rather than on the library. Measured across the 40 seeds: jitter raised solved 0.0 1 39 <- passes by a single seed locally; 0 on CI 1e-15 16 24 1e-12 20 20 1e-9 14 26 Only [0.0] is marginal, which is exactly the one parametrisation CI fails. Fix, keeping the regression this test exists for (never hand back NaN as though it were a valid reconstruction) fully intact: - The vacuity guard becomes `solved > 0` — at least one seed must actually have reached the finiteness assertion. That is what "not vacuous" means here; it is independent of how the runner rounds. - "The band really is degenerate" is now pinned on the fixture instead, via `cond(ZTZ) > 1e12`. Measured range is 1.07e16 to 3.71e18 across all seeds and jitter values, so the margin is wide and the property is deterministic. Verified in both directions by simulation: with no seed raising (the CI condition) all four parametrisations now pass; with every seed raising (genuine vacuity, finiteness never checked) the test still fails. Full suite: 906 passed, 52 skipped. The 3 test_transformer failures are pre-existing and unrelated (missing optional pynufft). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KazMzMZYPLfaZoYQ79YQ8Q --- .../util/test_cholesky_degenerate.py | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/test_autoarray/util/test_cholesky_degenerate.py b/test_autoarray/util/test_cholesky_degenerate.py index a142793b..5d488506 100644 --- a/test_autoarray/util/test_cholesky_degenerate.py +++ b/test_autoarray/util/test_cholesky_degenerate.py @@ -107,11 +107,16 @@ def test__fnnls_cholesky__never_returns_a_non_finite_solution(jitter): # The producer regression: across the whole near-degenerate band, the # solver must either return a finite solution or raise -- never hand back # NaN as though it were a valid reconstruction. - raised = 0 + solved = 0 for seed in range(40): ZTZ, ZTx = _normal_equations(n=12, n_data=40, jitter=jitter, seed=seed) + # The band really is degenerate. This is a property of the fixture, so it + # holds on every machine -- unlike *how the solver responds* to it, which + # is decided by floating-point summation order (see the module comment). + assert np.linalg.cond(ZTZ) > 1.0e12 + try: P_initial = np.linalg.solve(ZTZ, ZTx) > 0 except np.linalg.LinAlgError: @@ -120,14 +125,24 @@ def test__fnnls_cholesky__never_returns_a_non_finite_solution(jitter): try: reconstruction = fnnls_cholesky(ZTZ, ZTx.T, P_initial=P_initial) except np.linalg.LinAlgError: - raised += 1 continue + solved += 1 + assert np.all(np.isfinite(reconstruction)) - # Sanity: the degenerate band must actually be exercising the guard, - # otherwise the assertion above is passing vacuously. - assert raised > 0 + # Sanity: at least one seed must have reached the finiteness assertion above, + # otherwise this test passes vacuously. + # + # NOT `raised > 0`. The invariant here is the same one + # `test__cholinsertlast__singular_insertion_never_yields_an_unusable_pivot` + # states: either raise, or return something finite. Raising is one *allowed* + # outcome, not a required one, so requiring it of some seed asserts on which + # side of zero the runner's BLAS happens to round the Schur complement. At + # `jitter=0.0` exactly one of these 40 seeds raises on a single-threaded local + # BLAS and none do on the CI runners, so `raised > 0` was a coin flip that + # failed CI on a green library. + assert solved > 0 def test__fnnls_cholesky__well_conditioned_problem_is_unaffected():