Skip to content

SupernodalLU: refine only for perturbed pivots, drop residual-check allocation#1111

Merged
ChrisRackauckas merged 2 commits into
SciML:mainfrom
ChrisRackauckas-Claude:supernodal-lu-fixes
Jul 25, 2026
Merged

SupernodalLU: refine only for perturbed pivots, drop residual-check allocation#1111
ChrisRackauckas merged 2 commits into
SciML:mainfrom
ChrisRackauckas-Claude:supernodal-lu-fixes

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

⚠️ Draft — please ignore until reviewed by @ChrisRackauckas.

Two solve-path fixes found by auditing the vendored src/SupernodalLU against PureKLU.jl's optimization history, plus one recorded negative result. Independent of #1109; touches solve.jl, numeric.jl (comment only), and the sparse ext.

1. Refine only for perturbed pivots

_auto_refine refined whenever the factor was MC64-matched, even with zero perturbed pivots. But matching improves conditioning — refinement exists to recover what static pivot perturbation loses, so a matched-but-unperturbed factor needs none. Measured cost of the redundant work:

matrix relresid refine=0 refine=:auto time refine=0 refine=:auto
poisson40 forced-match 1.15e-15 7.04e-16 123.3 µs 273.1 µs (2.22×)
weakdiag40 auto-match 9.06e-16 5.81e-16 123.8 µs 273.9 µs (2.21×)
reverse-perm n=1600 7.86e-17 5.06e-17 234.6 µs 480.9 µs (2.05×)

Every ODE/Newton solve on a matched factor was paying ~2.1× for a residual change nobody asked for. Now nperturbed > 0 ? 3 : 0; a genuinely perturbed factor still refines (both directions tested).

2. Allocation-free residual check

The ext's post-solve residual check on a perturbed factor allocated a fresh n-vector per solve (r = F.A * y, 8n+104 B). Now mul!(F.ir_r, F.A, y) into the factor-owned buffer — that path measures 0 allocations (was ~2.5 kB at n=300).

3. Negative result (documented, not applied)

Narrowing bcaches from Vector{Any} to Vector{Union{Nothing,C}} so Julia union-splits it into a static call: measured 2.3–2.7 % slower (poisson3d_28 0.1720s vs 0.1681s; poisson2d_256 0.0907s vs 0.0883s) and it breaks init_cacheval's cacheval type-pinning — the empty prototype has no caches, so its element type can never match a real factorization's, giving a setfield! TypeError on every solve through the LinearSolve API. The dispatch it targets is tens of calls per factorization against hundreds of ms of GEMM, and the solve path never touches the field (already @inferred-clean). Recorded in the field comment so it isn't retried, in the spirit of PureKLU's #18/#19/#21.

The audit's larger finding — _solve_panels! is 96 % of solve time and its BLAS calls on ~12×12 panels are mostly dispatch overhead, worth −34 % to −46 % — is deliberately not here; it will be its own PR.

GROUP=Core passes.

🤖 Generated with Claude Code

https://claude.ai/code/session_017rr42T1vFRRT8D7DMAmJzf

…llocation

Two solve-path fixes from an allocation/perf audit against PureKLU's
optimization history, plus one recorded negative result.

1. _auto_refine refined whenever the factor was MC64-matched, even with
   zero perturbed pivots. Matching improves conditioning, so refinement
   there is redundant: it measured 2.0-2.2x cost per solve for a
   residual change of ~1.5e-15 -> 7e-16. Now it triggers on
   nperturbed > 0 only, which is what refinement exists for. Every
   ODE/Newton solve on a matched factor was paying this. A matched
   n=1600 system solves to 8.4e-17 with zero refinement steps.

2. The ext residual check on a perturbed factor allocated a fresh
   n-vector per solve (r = F.A * y, 8n+104 bytes). It now writes into
   the factor-owned F.ir_r via mul!, making that path 0 allocations
   (was ~2.5 kB at n=300).

3. Negative result, documented in the bcaches field comment so it is
   not retried: narrowing bcaches from Vector{Any} to
   Vector{Union{Nothing,C}} for union-splitting measured 2.3-2.7%
   SLOWER (poisson3d_28 0.1720s vs 0.1681s; poisson2d_256 0.0907s vs
   0.0883s) and breaks init_cacheval's cacheval type-pinning - the
   empty prototype has no caches, so its element type can never match a
   real factorization's, giving a setfield! TypeError on every solve
   through the LinearSolve API. The dispatch involved is tens of calls
   per factorization against hundreds of ms of GEMM; the solve path
   never touches the field and is already @inferred-clean.

Regression tests added for both fixes.

GROUP=Core passes.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… zero

The test asserted 0 allocations for a solve through the LinearSolve
cache, but each solve! returns a small LinearSolution whose allocation
the compiler elides only on some versions/platforms - it measured 0
locally and 80 B on CI (Downgrade and the lts test job). The test now
asserts < 256 B, which still catches the regression it exists for by an
order of magnitude: the old code allocated a fresh n-vector per solve
(8n+104 = 2504 B at n=300) on top of that.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

CI triage on the first run — 3 failures, 2 mine and now fixed, 1 not reachable from this diff:

Mine (fixed in the pushed commit): the allocation-free residual check test asserted exactly 0 allocations for a solve through the LinearSolve cache. But each solve! returns a small LinearSolution whose allocation the compiler elides only on some versions/platforms — it measured 0 locally and 80 B on CI (Downgrade + the lts test job). The test now asserts < 256 B, which still catches the regression it exists for by an order of magnitude: the old code allocated a fresh n-vector per solve on top of that (8n+104 = 2504 B at n=300).

Not mine: NonlinearSolve.jl/Core failed two robustness cases (Watson alg #2: 3.32 ≤ 1e-3; Brown almost linear alg #4: 5.5 ≤ 1e-3). Every line this PR changes is inside SupernodalLU's solve path or its ext solve!, which run only when SupernodalLUFactorization is the selected algorithm — and this PR does not change the default (that is #1109). Those NonlinearSolve cases are small dense nonlinear systems, so they cannot reach this code. The same downstream job passes on #1109, #1112 and on main. I could not re-run the job to confirm flakiness (no admin rights on the repo), so the fresh run triggered by this push is the second sample — I will report what it shows rather than assume.

@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review July 25, 2026 13:39
@ChrisRackauckas
ChrisRackauckas merged commit 9e21194 into SciML:main Jul 25, 2026
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants