Skip to content

Commit 2f62140

Browse files
author
Abhishek Bagusetty
committed
test_scipy_sparse_linalg: assert wide_spectrum against per-solver criterion
Reverts the previous loose 1e-3 ||r||/||b|| bound; asserts each solver against its own contractual stopping criterion instead. cg -> ||r|| / ||b|| < 10 * rtol minres -> ||r|| / (||A|| ||x||) < 10 * rtol Both bounds verified against SciPy 1.15 on the same problem: cg stops at iter 48 with ||r||/||b|| ~ 7e-9, minres stops at iter 40 with ||r||/(||A|| ||x||) ~ 2e-7 -- the prior 1e-5 bound on ||r||/||b|| was unreachable for minres on this matrix (||A||~1e2 inflates ||r||/||b|| by ~||A|| ||x||/||b||~1.4e2 relative to the criterion minres actually optimises). Avoids SVD by using max(|diag|) for ||A||_2 of the diagonal matrix.
1 parent 943575f commit 2f62140

1 file changed

Lines changed: 28 additions & 11 deletions

File tree

dpnp/tests/test_scipy_sparse_linalg.py

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,22 +1169,39 @@ def test_wide_spectrum_diagonal(self, solver):
11691169
# n=30; broader spectra need maxiter > 1000 and are not
11701170
# useful for a smoke test.
11711171
#
1172-
# The assertion threshold is sized for MINRES, whose
1173-
# SciPy-matching stopping criterion is ``||r|| / (||A||
1174-
# ||x||) <= rtol`` rather than ``||r|| / ||b|| <= rtol``.
1175-
# With ||A|| ~ 1e2 here, ||r|| / ||b|| converges to roughly
1176-
# ``rtol * Anorm * ynorm / ||b||``, which for rtol=1e-7 on
1177-
# this system lands around 1e-4. CG (which actually tests
1178-
# ``||r|| / ||b||``) reaches ~1e-7 comfortably, so the
1179-
# bound below is dominated by MINRES.
1172+
# The assertion uses each solver's own contractual stopping
1173+
# criterion rather than a single ``||r|| / ||b||`` bound:
1174+
# CG (and SciPy's cg) tests ``||r|| / ||b|| <= rtol``, while
1175+
# MINRES (matching SciPy) tests ``||r|| / (||A|| ||x||) <=
1176+
# rtol``. With ||A|| ~ 1e2 on this matrix the two ratios
1177+
# differ by ~||A|| ||x|| / ||b|| ~ 1.4e2, so asserting a
1178+
# single ``||r|| / ||b||`` bound that holds for both would
1179+
# require either tightening ``rtol`` (more iterations for no
1180+
# algorithmic benefit) or a slack constant that hides real
1181+
# regressions. Using the solver's own criterion with a small
1182+
# safety factor catches genuine accuracy drift without
1183+
# baking in the ||A|| ||x|| / ||b|| ratio of this particular
1184+
# input. Verified against SciPy 1.15: both solvers stop at
1185+
# iter 48 (cg) and iter 40 (minres) with the residuals below.
11801186
n = 30
11811187
diag = numpy.logspace(-1, 2, n, dtype=numpy.float64)
11821188
ia = dpnp.asarray(numpy.diag(diag))
11831189
ib = _rhs(n, dpnp.float64)
1184-
x, info = solver(ia, ib, rtol=1e-7, maxiter=2 * n)
1190+
rtol = 1e-7
1191+
x, info = solver(ia, ib, rtol=rtol, maxiter=2 * n)
11851192
assert info == 0
1186-
res = float(dpnp.linalg.norm(ia @ x - ib) / dpnp.linalg.norm(ib))
1187-
assert res < 1e-3
1193+
r_norm = float(dpnp.linalg.norm(ia @ x - ib))
1194+
if solver is cg:
1195+
# cg checks ||r|| / ||b||; SciPy reaches ~7e-9 at rtol=1e-7.
1196+
b_norm = float(dpnp.linalg.norm(ib))
1197+
assert r_norm / b_norm < 10 * rtol
1198+
else:
1199+
# minres checks ||r|| / (||A|| ||x||); SciPy reaches ~2e-7.
1200+
# ``ia`` is diagonal here, so ||A||_2 == max(|diag|);
1201+
# computing it that way avoids an unnecessary 30x30 SVD.
1202+
A_norm = float(diag.max())
1203+
x_norm = float(dpnp.linalg.norm(x))
1204+
assert r_norm / (A_norm * x_norm) < 10 * rtol
11881205

11891206
@pytest.mark.skipif(not has_support_aspect64(), reason="fp64 is required")
11901207
@pytest.mark.parametrize("solver", [cg, gmres, minres])

0 commit comments

Comments
 (0)