Tests: growth_tol for LDLᵀ, seeds, SparseMatrixCSC constructors, blocked regime-B coverage, refinement bound

[michel2323]

Refs #86, #63, #76, #79, #85, #89

Task: Post-T21 clean-up PR 2: test-suite hardening (STATE.md §3, "Tests and docs"). Tests only, no library code.

What was built:

• test/utils.jl: factor_growth now handles 2×2 pivots and perturbed pivots. The old denominator min |d[1:n]| is 0 on the diagonal of a 2×2 block (KKT with δ = 0), so growth_tol was Inf on every 2×2 case and checked nothing. A 2×2 block now contributes |det| / ‖block‖_F, a lower bound on its smallest singular value; the smaller determinant of the symmetric and Hermitian forms is used. Perturbed (±ε) pivots are left out of the minimum because they are compared bitwise. LU is unchanged: its cases have no 2×2 blocks and no perturbed pivots (checked with pivot_stats).
• test/test_numeric_ldlt.jl (Device LDLᵀ differs from ref_ldlt! on the K2 dumps (pivot sequence, nperturbed; inertia equal) #86):
  • Every elementwise panel/D comparison with the reference (default, regime-A/Int64, perturbation, regime C, pivot_type D/N, API "diag") uses growth_tol(T, Nr).
  • ldlt_error(A, S, Nh) ≤ tol(T) is added next to them.
  • piv, pivot_kind, the perturbed D entries and the ±ε value are still compared exactly.
• New testset "regime B on the blocked path" (perf: regime-C LDLᵀ through blocked pivot steps and GEMMs (#75) #89):
  • Matrix: KKT 1e-3 interleaved, with regime_c_width = REGIME_B_MAX_WIDTH, regime_c_rows = 1024.
  • Asserts that some front has regime == REGIME_B && ldlt_blocked_path && !takes_c_path and that such a front contains 2×2 pivots.
  • Compares piv, pivot_kind, D, panels, stats and totals with the reference, plus ldlt_error and a solve.
  • Side finding: the default analysis of the existing "kkt 1e-3 H, δ = 0, interleaved (2×2)" case already reached this branch on every element type, but nothing asserted it.
• Seeds (T15: device LDLᵀ/LDLᴴ in regimes A/B/C, diagonal solve, pivot statistics #76, T19: general LU ("G"): reference, device kernels in regimes A/B/C, LU solves with A/Aᵀ/Aᴴ, lu/lu!, report #85): Random.seed!(666) now opens every testset that draws random numbers in test_numeric_{cholesky_a,b,c,ldlt,lu}, test_solve, test_refinement, test_ubatch, test_schur, test_matching, test_reference_{cholesky,ldlt} and test_fgmres. "Draws" includes the generators in matrices.jl.
• test/test_api.jl (T13: public API v0.1 (DirectSolver, execute! phases, generic Cholesky, ported tests) #63): new testset for DirectSolver(::SparseMatrixCSC, structure, view; index) and update!(solver, ::SparseMatrixCSC) on the CPU backend.
  • Views L/U/F, index bases O/Z.
  • Solve and refactorization to tol(T).
  • InvalidValueError for a wrong size, a different nnz, a different index type and a rectangular matrix.
• test/test_refinement.jl (T16: iterative refinement, solve_mode, interrupt, logging #79): @test_broken r1 <= r0 / 100 is replaced by r0 ≤ 100 eps and r1 ≤ r0. Measured on CPU over 16 seeds: r0 ≈ 1.3 eps, and one step gains 3× to 5.4×. The two T16 Report sentences that named the @test_broken are updated; that is the only TASKS.md edit.

Tests:

• CPU: SDS_TEST_GPU=0 julia --project=. -e 'using Pkg; Pkg.test()' (Julia 1.13.1, KA CPU backend, 4 threads): 69799 pass / 0 fail / 0 broken, 23m48s. Before this PR there was 1 broken, the T16 @test_broken. No "Method definition … overwritten" warnings in the log (item 7: nothing to remove). After the review fix (e8864b7), test_solve + test_numeric_cholesky_a re-run on CPU: 1446 / 1446 pass.
• CUDA (CI test-gpu, SDS_TEST_CPU=0 SDS_TEST_SKIP=test_aqua, head e8864b7): 76084 pass / 0 fail / 0 broken, 18m20s.

Measured on CPU with the new growth (Float32/Float64/ComplexF32): device-vs-reference D and panel errors are ≤ 0.05·panel_tol everywhere, and ldlt_error is ≤ 0.02·tol(T). Growth is ≈ 1 on random_symindef, so nothing is loosened there. On high-growth KKTs the growth is large (Float32 kkt 1e-8: 2.5e8), so the elementwise check becomes weak there and ldlt_error is the effective check. That is the same trade-off as test_numeric_lu.

Item 6 (SPLIT_FILES): no change. test_numeric_lu is already in SPLIT_FILES (#93), and every testset loops over ELTYPES. Per-part wall times in this run, which ran in parallel with other files: Float64 10.8 s, ComplexF64 20.1 s, Float32 233.6 s, ComplexF32 336.2 s.

Deviations from PLAN.md / TASKS.md: none. The factor_growth change is test-only and is documented in its docstring.

Follow-up issues opened: none.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Wzx8WtMEXnAfyzyujdySFh

---

Generated by Claude Code

[claude]

VERDICT: CHANGES_REQUESTED (head ec646ac)

Scope. Owner clean-up PR ("Post-T21 clean-up PR 2", STATE.md §3 "Tests and docs"), tests only: no src/, no ext/, no Manifest.toml, PLAN.md untouched. The only TASKS.md edit updates the two T16 Report sentences that named the removed @test_broken, as the description says. All six STATE.md §3 test items (#86, #63, #76/#85, #79, #89, item 6/7) are addressed.

Checked.

• factor_growth: d[n + k] is the 2×2 subdiagonal in the reference layout (src/reference/ldlt.jl, d_error docstring); |det| / ‖B‖_F ≤ σ_min, and the smaller of the symmetric/Hermitian determinants only loosens, never tightens, the bound. The LU reference also carries pivot_kind (PIVOT_KIND_PERTURBED), so excluding perturbed pivots can only tighten growth_tol in test_numeric_lu.jl; the LU cases that use it carry none.
• test_numeric_ldlt.jl: every panel_tol → growth_tol swap keeps piv, pivot_kind, the perturbed D entries and ±ε exact, and adds ldlt_error ≤ tol(T) as the growth-independent check (same trade-off as T19 for LU). The new "regime B on the blocked path" testset uses existing predicates (ldlt_blocked_path, takes_c_path, sncols); with regime_c_width = REGIME_B_MAX_WIDTH such fronts cannot take the C path, and the schedule is backend-independent so !isempty(blocked_b) cannot flip between CPU and CUDA. Testset loops over BACKENDS and ELTYPES, seeded.
• test_api.jl: the CSC constructor and update!(::SparseMatrixCSC) paths (src/solver.jl, src/matrix.jl) raise InvalidValueError for every case asserted (size/nnz, index type via A isa M, rectangular via the square check). Loops over ELTYPES/INTTYPES, CPU only by design, seeded.
• test_refinement.jl: @test_broken replaced by r0 ≤ 100 eps and r1 ≤ r0; nothing weakened, no @test_broken left in the suite.
• Random.seed!(666) additions in 13 files.
• CI: build green, CPU and CUDA test jobs pending (handled by the pipeline). I could not execute Julia in this review sandbox.

Blocking.

1. Report vs diff: the description claims Random.seed!(666) "now opens every testset that draws random numbers" in the listed files, but four such testsets are still unseeded: test_solve.jl "solve plan and workspace" (l. 18), "residuals, all regimes" (l. 77), "deterministic and atomic variants" (l. 124), and test_numeric_cholesky_a.jl "plan: regime-A groups" (l. 33). All draw random_spd (and solve_rhs). Add the seeds (one line each) or correct the sentence. Inline comments on test/test_solve.jl and test/test_numeric_cholesky_a.jl.

Non-blocking nits (inline): r1 ≤ r0 is empirical at rounding level, watch the CUDA margin; d_error docstring still points at panel_tol.

[claude]

VERDICT: APPROVE (head e8864b7)

Reviewed against STATE.md §3 "Tests and docs" (the task text this clean-up PR implements), AGENTS.md and PLAN.md.

Scope. Tests and test helpers only; no library code, no PLAN.md, no Manifest.toml. The only TASKS.md edit is inside the T16 Report (status line and the deviation sentence that named the removed @test_broken), which keeps that Report truthful. Public names, parameter strings and phase strings untouched.

Tests. Every item of the "Tests and docs" list that is a test item is covered:

• test_numeric_ldlt.jl moves every device-vs-reference D/panel comparison to growth_tol(T, Nr) and adds ldlt_error ≤ tol(T) next to each; piv, pivot_kind, perturbed D entries and ±ε stay exact (#86).
• factor_growth (test/utils.jl): |det| / ‖·‖_F ≤ σ_min is a valid lower bound for a 2×2 block, taking the smaller of the symmetric and Hermitian determinants is conservative, and excluding perturbed pivots only tightens the LU tolerance (LU Nr.pivot_kind is written by _lu_pivot!, so the helper is well defined there). Previously min |d[1:n]| was 0 on δ = 0 KKT 2×2 blocks, so growth_tol was Inf there; this PR makes those checks real.
• New "regime B on the blocked path" testset: the predicate regime == REGIME_B && ldlt_blocked_path && !takes_c_path matches the definitions in src/symbolic/layout.jl/schedule.jl; kkt_matrix is Hermitian for complex T, so the default herm = true of ldlt_error is correct; loops over BACKENDS and ELTYPES (#89).
• Random.seed!(666) opens every random testset in the 13 files listed in the PR body (#76, #85).
• test_api.jl: DirectSolver(::SparseMatrixCSC, …; index) and update!(::SparseMatrixCSC) on the CPU backend, views L/U/F, bases O/Z, solve + refactorization to tol(T), and InvalidValueError for wrong size, different nnz, different index type (A isa M check) and a rectangular matrix (the "must be square" check in the CSR constructor) (#63).
• test_refinement.jl: the @test_broken is replaced by r0 ≤ 100 eps and r1 ≤ r0 (#79). Not weakened: the 100× reduction is still asserted on the perturbed LDLᵀ case.
• No @test_broken remains; tolerances come from tol/panel_tol/growth_tol in test/utils.jl.

Report. Matches the diff: the described changes, the "no follow-up issues" statement and the SPLIT_FILES assessment are consistent with the code. CPU counts (69799 pass / 0 broken) are plausible; CUDA is pending CI on this PR, as stated.

CI. All four checks are still pending at review time; handled by the pipeline.

Blocking findings: none.

Non-blocking: one inline nit on r1 <= r0 in test_refinement.jl (the atomic forward sweep on CUDA makes the two residuals run-dependent; deterministic_mode = 1 would make the assertion reproducible).