Skip to content

Fold the SparseArrays extension into the root module - #1191

Merged
ChrisRackauckas merged 1 commit into
SciML:mainfrom
ChrisRackauckas-Claude:sparsearrays-fold
Aug 9, 2026
Merged

Fold the SparseArrays extension into the root module#1191
ChrisRackauckas merged 1 commit into
SciML:mainfrom
ChrisRackauckas-Claude:sparsearrays-fold

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Aug 9, 2026

Copy link
Copy Markdown
Member

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

What changed and why

ext/LinearSolveSparseArraysExt.jl moves to src/sparsearrays.jl and is included from
src/LinearSolve.jl. SparseArrays is an unconditional [deps] entry — src/SupernodalLU
does using SparseArrays at package load time — so the extension always loaded, but it
loaded after the root module precompiled. Inserting its init_cacheval methods
invalidated the default-path specializations the root @compile_workload had just cached,
so that coverage was thrown away before any user reached it.

Measured, using LinearSolve alone, RecursiveFactorization not loaded, Julia 1.12.6:

before after
first solve(prob), 4×4 dense Float64 (median of 7 fresh processes) 82.0 ms 0.087 ms
unique invalidated MethodInstances on using LinearSolve 110 90
@elapsed using LinearSolve (min of 7) 1.361 s 1.360 s

Invalidation evidence

SnoopCompileCore.@snoop_invalidations using LinearSolve, min-identical across two runs per
arm. The four trees rooted at the extension's init_cacheval are gone; nothing else moves
except SparseColumnPivotedQRAMDExt shedding 5 children.

########## before (origin/main @ 954ea58a) ##########
=== total unique invalidated MethodInstances: 110
nchildren reason      method
1         inserting   LinearSolveSparseArraysExt.init_cacheval @ LinearSolveSparseArraysExt.jl:891
1         inserting   LinearSolveSparseArraysExt.init_cacheval @ LinearSolveSparseArraysExt.jl:759
1         inserting   LinearSolveSparseArraysExt.init_cacheval @ LinearSolveSparseArraysExt.jl:1012
13        inserting   LinearSolveSparseArraysExt.init_cacheval @ LinearSolveSparseArraysExt.jl:581
36        inserting   SparseArrays._mapreduce @ sparsevector.jl:1667
54        inserting   SparseColumnPivotedQRAMDExt._amd_colperm @ SparseColumnPivotedQRAMDExt.jl:27
(+ 2 small SparseArrays trees, + 7 zero-child rebindings)

########## after (this branch) ##########
=== total unique invalidated MethodInstances: 90
nchildren reason      method
36        inserting   SparseArrays._mapreduce @ sparsevector.jl:1667
49        inserting   SparseColumnPivotedQRAMDExt._amd_colperm @ SparseColumnPivotedQRAMDExt.jl:27
(+ 2 small SparseArrays trees, + 7 zero-child rebindings)

TTFX, two independent runs of 7 fresh processes per arm. Run 1 is the headline above; run 2
alternates base/branch rep-by-rep so machine load hits both equally (host load average ~39
throughout). Raw @elapsed of the first solve(prob), in ms:

run 1  base: 83.7  81.4  84.1  81.7  82.6  81.9  82.0   (median 82.0, min 81.4)
       fold:  0.083 0.101 0.103 0.072 0.073 0.100 0.087 (median 0.087, min 0.072)

run 2  base: 87.2  88.5  87.8  91.2  84.7  107.0 90.5   (median 88.5, min 84.7)
       fold:  0.200 0.134 0.107 0.152 0.105 0.163 0.150 (median 0.150, min 0.105)

Run 2 is uniformly a few percent slower on both arms (busier host); the ratio is ~550-950x
either way.

Precompile cost, same paired protocol, JULIA_NUM_PRECOMPILE_TASKS=1, 4 reps
(a unique comment is appended to src/LinearSolve.jl each rep because Julia ≥ 1.11
content-hashes sources):

metric (min over reps) before after delta
CPU, user+sys incl. children 37.56 s 37.31 s −0.25 s
wall 24.99 s 22.79 s −2.2 s
Pkg's own lines LinearSolve 15.4 s + → SparseArraysExt 6.0 s LinearSolve 19.4 s −2.0 s

Folding is precompile-neutral on CPU and slightly cheaper on wall: the root module gets
bigger, but there is no longer a second module to load, hash and write.

What moved, and how to move it back

src/sparsearrays.jl is the extension file verbatim apart from its module header and its
using LinearSolve: ... import list. Git sees it as a rename (98% similarity), so the review
diff is 64 added lines, not 1500. It is deliberately one self-contained unit, not
scattered into the existing src/ files, and its header states the excise-later path:

move this file to ext/, wrap it in module LinearSolveSparseArraysExt ... end with a
using LinearSolve: ... import list, point its include back at ../src/KLU/klu.jl,
restore the getcolptr/rowvals/nonzeros stubs and drop the include in
src/LinearSolve.jl, restore the [extensions] entry, and re-add SparseArrays to the
trigger lists of the fourteen extensions that co-trigger on it.

Placement in src/LinearSolve.jl is load-bearing in both directions: after the
needs_square_A / useblis / usecuda traits, which the file's own @compile_workload
calls, and before the root @compile_workload, which it would otherwise invalidate.

Project.toml

LinearSolveSparseArraysExt is deleted from [extensions], and SparseArrays is removed from
the trigger list of the fourteen extensions that co-triggered on it (CUSOLVERRF, CliqueTrees,
Enzyme, Ginkgo, HSL, MUMPS, PETSc, PETScMPI, ParU, PureUMFPACK, Pardiso, SuperLUDIST,
STRUMPACK, Sparspak). Correction to what I was told going in: SparseArrays was in [deps]
and [extensions] but never in [weakdeps], so those fourteen entries were already no-ops —
a trigger that names a hard dep is always satisfied. Removing them is tidying, not a
prerequisite. SparseArrays stays in [deps] and keeps its [compat] entry.

Collateral, all forced by the move

  • getcolptr/rowvals/nonzeros stubs deleted from src/LinearSolve.jl. They collided
    with the moved file's using SparseArrays: ..., getcolptr, rowvals, nonzeros. They were
    dead: nothing in src/, ext/, lib/, test/ or docs/ ever called
    LinearSolve.getcolptr and friends, and the extension never defined methods on them —
    src/SupernodalLU imports the real ones from SparseArrays directly.
  • LinearSolveSparseArraysExt.KLULinearSolve.KLU in test/Core/basictests.jl,
    test/qa/qa.jl, docs/make.jl and docs/src/solvers/solvers.md. src/KLU/klu.jl was
    already in src/; it was included into the extension over ../src/.
  • init_cacheval(::NormalCholeskyFactorization, ...) loses its where {T <: BLASELTYPES}
    in favour of inline <:BLASELTYPES bounds. T was unbound on the AnyGPUArray member of
    the A union and unused in the body; Aqua's test_unbound_args only scans the package
    module, so it never saw this method while it lived in an extension. Dispatch is unchanged
    for every sparse case; the only widening is that Symmetric's eltype and its parent's
    eltype are no longer tied to each other, which Symmetric{T, S<:AbstractMatrix{<:T}}
    already guarantees.
  • const libamd/libbtf added to src/KLU/klu.jl. JET's report_package(LinearSolve; target_modules = (LinearSolve,)) now reaches LinearSolve.KLU and reported 30 findings of
    the form `LinearSolve.KLU.libamd` is not defined. Upstream KLU.jl does not define
    them either — every AMD/BTF wrapper in src/KLU/wrappers.jl is a latent UndefVarError.
    Two constants in the same :libklu style fix the bindings. The wrappers stay uncalled, so
    I have not verified that they now work end to end, only that the bindings resolve.

Verification

Julia 1.12.6, x86_64 Linux, OpenBLAS. Commands and output tails:

$ GROUP=Core julia +1.12 --project -e 'using Pkg; Pkg.test()'
Basic Tests   |  766    766  10m07.8s
Default Alg Tests |  127    127  6m55.6s
ForwardDiff Overloads |  147    147  5m26.4s
Adjoint Sensitivity |  107    107  2m11.6s
... 31 test sets, 0 failures, 0 errors
     Testing LinearSolve tests passed

$ GROUP=QA julia +1.12 --project -e 'using Pkg; Pkg.test()'
JET Tests                  |   34       8     42  1m25.2s   (8 pre-existing broken markers)
Allocation QA              |   60            60  1m57.4s
SupernodalLU Allocation QA |    8             8     21.0s
Quality Assurance          |   48            48  5m04.6s
     Testing LinearSolve tests passed

Quality Assurance includes Aqua's ambiguity and piracy checks, which matter here because the
move changes which module owns these methods: Method ambiguity 1/1, Piracy 1/1, Unbound type
parameters 1/1, ExplicitImports 6/6, Public API documentation 2/2
— all green (the unbound
one only after the NormalCholeskyFactorization fix above; it fails without it, see below).

$ GROUP=Core julia +1.10 --project -e 'using Pkg; Pkg.test()'
... 31 test sets, 0 failures, 0 errors
     Testing LinearSolve tests passed

$ GROUP=DefaultsLoading julia +1.12 --project -e 'using Pkg; Pkg.test()'
Defaults Loading Tests |   11     11  59.0s      Testing LinearSolve tests passed

$ GROUP=Preferences julia +1.12 --project -e 'using Pkg; Pkg.test()'
Dual Preference System Integration |  103    103  13.1s      Testing LinearSolve tests passed

$ julia +1.12 --project=docs docs/make.jl
[ Info: HTMLWriter: rendering HTML pages.
(exit 0; the only warnings are the pre-existing solvers.md size and missing-docstring ones)

$ julia +1.12 --project=@runic -e 'using Runic; exit(Runic.main(["--check","src/","test/","docs/make.jl","ext/","lib/"]))'
(exit 0)

$ typos .
(exit 0)

Failing-before / passing-after for the QA fixes

The three QA findings above are all caused by the move (they are checks that only apply to
the package module), so they discriminate. On the first GROUP=QA run of this branch, before
the fixes:

Unbound type parameters: Test Failed at Aqua/src/unbound_args.jl:38
  Evaluated: isempty(Method[init_cacheval(alg::NormalCholeskyFactorization,
             A::Union{Symmetric{T, <:AbstractSparseArray{T}}, AbstractSparseArray{T},
             AnyGPUArray}, ...) where T<:Union{Float32, Float64, ComplexF64, ComplexF32}
             @ LinearSolve src/sparsearrays.jl:1065])

Quality Assurance: JET-test failed
  ═════ 30 possible errors found ═════
  `LinearSolve.KLU.libamd` is not defined ... (× 24)
  `LinearSolve.KLU.libbtf` is not defined ... (× 6)

Quality Assurance: Error During Test
  LoadError: UndefVarError: `klu_mod` not defined     (test/qa/qa.jl:263)

ERROR: Package LinearSolve errored during testing

After: Quality Assurance | 48 48, Testing LinearSolve tests passed (quoted above).

Pre-existing failure this PR does not cause

GROUP=QA on the 1.10 LTS is red on unmodified origin/main too:

JET Tests for Sparse Factorizations: JET-test failed at test/qa/jet.jl:136
  Expression: JET.@test_opt solve(prob_sparse, KLUFactorization())
  ═════ 2 possible errors found ═════
  ... -> SciMLLogging.emit_message -> Base.CoreLogging.env_override_minlevel
      -> moduleroot -> sprint(show, ::String) -> pairs(::NamedTuple) -> typejoin
  │ runtime dispatch detected: Base.UnionAll(%403::Any, %405::Any)::Any
JET Tests | 20 pass  1 fail  21 broken  42

I ran GROUP=QA on 1.10 against both origin/main and this branch. The two reports are
byte-identical apart from one stack frame naming the file that moved
(LinearSolveSparseArraysExt.jl:518src/sparsearrays.jl:531), and the testset totals are
the same 20/1/21/42. The whole reported dispatch chain is inside Base.CoreLogging and
Base.typejoin. 1.12 is clean, and it is invisible to CI (SciML/.github's matrix script
clamps QA to ["1"], so no QA (julia lts) job is ever emitted) — it only bites local LTS
runs.

Tracked as #1190. Bisected to
#1083, which removed a broken = true that had
been covering a pre-existing Base-1.10 inference limitation: @test_opt and log emission are
mutually exclusive on 1.10 for any package. Zero-SciML reproducer, in an environment holding
only JET: f() = (@info "hello"; nothing); JET.@report_opt f() gives 2 reports on 1.10.11 and
0 on 1.11.9 and 1.12.6. Not this PR's to fix.

CI

55 pass, 0 fail at the time of writing; 4 still running (ModelingToolkit,
SciMLSensitivity and BoundaryValueDiffEq downstream, and GPU). The jobs that cover what I
could not run locally are green, including every group whose [extensions] trigger list
this PR edited:

Core (lts, 1, pre)              QA (julia 1)                     Trim (julia 1)
DefaultsLoading (lts, 1, pre)   Preferences (lts, 1, pre)        Documentation
AppleAccelerate (lts, 1, pre)   Downgrade - Core                 Runic, Typos
LinearSolveElemental (1, lts)   LinearSolveMUMPS (1, lts)        LinearSolvePureUMFPACK (1, lts, pre)
LinearSolveGinkgo (1, lts)      LinearSolvePardiso (1, lts)      LinearSolveSuperLUDIST (1, lts)
LinearSolveHSL (1, lts)         LinearSolvePartitionedSolvers (1, lts)   LinearSolveSTRUMPACK (1, lts, pre)
LinearSolveHYPRE (1, lts)       LinearSolveParU (1, pre)         LinearSolvePETSc (1, lts)
Downstream: NonlinearSolve/Core, OrdinaryDiffEq/InterfaceII

Not verified

  • CUSOLVERRF has no CI group (its [compat] conflicts with CUDSS, see the note in
    test/qa/qa.jl), so its trigger-list edit is unexercised. It keeps "CUSOLVERRF" as a
    trigger and SparseArrays is a hard dep, so it should trigger exactly as before, but nothing
    proves it.
  • PETScMPI likewise has no green job here; only LinearSolvePETSc ran.
  • MKL, Windows, 32-bit — locally Linux + OpenBLAS only. macOS/AppleAccelerate, Trim,
    AD, Downgrade and the downstream jobs were not run locally either, but CI covers them
    and they are green (above). GPU is still running.
  • The libamd/libbtf ccall wrappers resolve their bindings now but are still never called,
    so "they work" is not claimed.
  • GROUP=QA on 1.10 cannot go green until the pre-existing JET failure above is resolved.

Things a reviewer should push back on

  1. Deleting LinearSolve.getcolptr/rowvals/nonzeros. Undocumented, unexported,
    error-throwing stubs with no callers anywhere in the repo — but they are technically a
    removal, and a downstream package could in principle name them.
  2. The NormalCholeskyFactorization signature widening. It is the one behavioural edit in
    this PR; I argue it is a no-op for dispatch, but it is a real signature change.
  3. Comment ratio. 29 of 64 added lines are comments, well over the 10% guideline. 18 of
    those are the mandated file header documenting the excise-later path, and 5 are edits to
    existing comments that named "the SparseArrays extension". Against the 1504-line file the
    header sits on, it is 1.2%. Happy to cut it further if you would rather it lived in the PR
    description only.
  4. The fourteen trigger-list edits are cosmetic, per the correction above. They can be
    dropped from this PR if you would rather keep the diff to the fold itself.

Relationship to #1188

#1188 ("Move default-path precompile coverage into the extensions that invalidate it") was
closed unmerged. Its sparse half is subsumed by this PR: it added a per-eltype
solve(denseprob) loop inside the SparseArrays extension to re-cache the default path that
extension invalidated. With SparseArrays in the root module there is nothing to re-cache — the
root workload's own solve(prob) now survives, which is the 82.0 ms → 0.087 ms above. No code
from #1188 is carried over here.

What #1188 raised and this PR does not address, left for you to decide:

  • RecursiveFactorization invalidates the default path independently, and genuinely is
    optional, so it cannot be fixed by folding. Move default-path precompile coverage into the extensions that invalidate it #1188 measured a first default solve(prob) at
    6.63 s with RF loaded and proposed a workload inside
    ext/LinearSolveRecursiveFactorizationExt.jl. I have not re-measured that number on this
    branch and have not implemented it; it deserves its own small PR judged on its own numbers.
  • Per-element-type (Float32) coverage in the root workload: a separate axis with its own
    measured precompile cost, unrelated to the fold.

🤖 Generated with Claude Code

SparseArrays is a hard `[deps]` entry -- `src/SupernodalLU` loads it at
package load time -- so `LinearSolveSparseArraysExt` always loaded, but it
loaded *after* the root module precompiled. Inserting its `init_cacheval`
methods invalidated the default-path specializations the root
`@compile_workload` had just cached, so the workload's `solve(prob)` coverage
was thrown away before any user reached it: a first `solve(prob)` on a 4x4
dense Float64 problem cost 82 ms rather than the 0.09 ms it costs once the
methods are in place before the workload runs.

`ext/LinearSolveSparseArraysExt.jl` moves to `src/sparsearrays.jl` unchanged
apart from its module header and import list, and is included just before the
root workload. It stays one self-contained file so it can be excised again if
SparseArrays ever becomes genuinely optional; the file header spells out the
steps. The `getcolptr`/`rowvals`/`nonzeros` stubs in `src/LinearSolve.jl` are
dropped -- nothing referenced them, and they collided with the moved file's
`using SparseArrays` import.

Project.toml drops the `LinearSolveSparseArraysExt` entry and removes
SparseArrays from the fourteen extensions that listed it as a co-trigger; it
was never in `[weakdeps]`, so those entries were already no-ops.
`LinearSolveSparseArraysExt.KLU` becomes `LinearSolve.KLU` in the tests, the
QA extension inventory and the docs.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review August 9, 2026 13:06
@ChrisRackauckas
ChrisRackauckas merged commit 2bef036 into SciML:main Aug 9, 2026
59 of 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