Add QR factorization (augmented sparse QR) for SparseWithDenseRowColMatrix#4
Closed
ChrisRackauckas-Claude wants to merge 1 commit into
Closed
Add QR factorization (augmented sparse QR) for SparseWithDenseRowColMatrix#4ChrisRackauckas-Claude wants to merge 1 commit into
ChrisRackauckas-Claude wants to merge 1 commit into
Conversation
…atrix `qr(A)` factors the bordered system [S U; V -I] with SuiteSparseQR — the numerically stable analogue of the augmented-LU path — returning a new `SparseWithDenseRowColQRAugmented`. Replaces the throwing `qr` stub. Design: ship augmented-QR only. A Woodbury-over-qr(S) mode shares the κ(S)·κ(C) cancellation of the LU Woodbury path (forming S⁻¹ is what loses accuracy, independent of LU vs QR) and is catastrophically inaccurate on ill-conditioned S, so `strategy = :woodbury` throws. Carries over: adjoint/transpose (lazily-cached separate qr of the adjoint, since SuiteSparseQR has no Qaug' \ b), complex-RHS-over-real split, r==0, and a LinearSolve algorithm (SparseWithDenseRowColQRFactorization, opt-in; default stays LU). Does not carry over (SuiteSparseQR limits, documented): symbolic-reuse refactor (qr!/refactor! re-factor fully), zero-allocation solve, and generic eltypes. Honest scope: augmented-QR is not more accurate than augmented-LU (LU can be better on ill-conditioned assembled A) and is slower on every axis; its value is being rank-revealing and the expected QR interface. README states this plainly. Two bugs found in adversarial review and fixed with regression tests: - Spurious SingularException on nonsingular ill-conditioned A: SuiteSparseQR's default tol=-2 deflation zeroed R-diagonal pivots; pass tol=0.0 and make the rank decision ourselves (genuine singularity still caught). - Float32/ComplexF32 sparse qr throws CHOLMODException on Julia 1.11 (works 1.10/1.12): restrict the QR path to Float64/ComplexF64 for consistent cross-version behavior; Float32/generic route to factorize/lu. Raise Aqua persistent_tasks tmax to 60s: no load-time task is spawned (verified), the default 10s wall-clock budget is flaky on loaded CI hosts. 297/297 tests pass on Julia 1.10, 1.11, and 1.12. Runic-formatted. No new deps. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
This was referenced May 30, 2026
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a QR factorization (
qr) forSparseWithDenseRowColMatrixAdds a QR factorization mirroring the existing LU/Woodbury path, as requested.
qr(A)builds a single sparse QR (SuiteSparseQR) of the bordered system[S U; V -I]— the numerically stable analogue of the existing augmented-LU path — returning a newSparseWithDenseRowColQRAugmented. The previousqrstub (which threw "not provided") is replaced.Design decision: augmented-QR only — no Woodbury-over-
qr(S)A design workflow plus direct measurement both concluded decisively: ship only the augmented QR. A Woodbury-over-
qr(S)mode shares the sameκ(S)·κ(C)cancellation as the LU Woodbury path — formingS⁻¹is what loses the accuracy, and that is independent of whetherS⁻¹comes from LU or QR. Measured forward error onA = S + U Vwith well-conditionedA(κ(A) ≈ 6) asκ(S)sweeps to3.6e14:qr(augmented)qr(S)(rejected)factorize :woodburyrefine=2So
qr(A; strategy = :woodbury)throws;:auto/:augmentedare the only modes.Honest scope — what QR does and does not buy you
This is the part worth scrutinizing. Augmented-QR is not more accurate than the existing augmented-LU path:
On well-conditioned
A, both sit at the noise floor (~1e-13/1e-16); the win is only over the Woodbury fast path.On ill-conditioned assembled
A, augmented-QR is backward-stable (forward error ~κ(A)·eps), and the augmented-LU path's partial pivoting is often more accurate on these structured matrices — measured orders apart forκ(A) ≳ 1e6.It is slower on every axis (banded interior +
rdense rows, factor/solve/refactor ms):QR's genuine, distinct value is being rank-revealing and providing the QR interface that users of almost-banded solvers (FastAlmostBandedMatrices) expect — not stability-over-LU and not throughput. The README states this plainly so nobody reaches for
qrexpecting it to be the accurate or fast option. If we want a feature that augmented-LU/Woodbury genuinely cannot do (least-squares for rank-deficientA), that is not a clean win via this bordered system — its least-squares solution is not the least-squares solution ofA x = b— so the path throwsSingularExceptionon singularA, matching the LU contract. Happy to revisit if you want least-squares semantics done properly.Optimization steps carried over (and the ones that don't)
Mᴴ ↔ Aᴴ; SuiteSparseQR has noQaug' \ b, so a separateqr(Maugᴴ)is built and lazily cached (QaugH), invalidated onrefactor!.r == 0degenerate caseqr(S).SparseWithDenseRowColQRFactorization()(opt-in; default stays the LU algorithm). SingularA→ReturnCode.Infeasible, KLU parity.refactor!qr!;refactor!/qr!re-factor fully. Documented; hot loops should uselu.ldiv!(::QRSparse, b); one allocation/solve.Float64/ComplexF64only — see below.Two bugs found in adversarial review and fixed (with regression tests)
SingularExceptionon nonsingular, ill-conditionedA. SuiteSparseQR's defaulttol = -2deflates ill-conditioned-but-nonsingular columns and writes an exact 0 onto the R-diagonal, soqr(A)threw on matricesfactorize/klusolve fine (reproduced atcond(A) = 1e12). Fixed by passingtol = 0.0to allqrcalls and making the rank decision ourselves; genuine singularity (exact-zero pivot) is still caught. Regression test added.Float32/ComplexF32sparseqrthrowsCHOLMODExceptionon Julia 1.11 (works on 1.10/1.12 — single-precision sparse QR is version-dependent). Rather than ship behavior that differs across Julia versions, the QR path is restricted toFloat64/ComplexF64, which work identically everywhere;Float32/generic eltypes route tofactorize/lu.Tests / CI
test/test_qr.jl(70 assertions): correctness vs BigFloat reference, the κ(S) stability sweep, the ill-conditioned-assembled-Ano-spurious-throw regression, eltype rejection, adjoint/transpose with lazy-cache checks, matrix RHS,refactor!/qr!, singularity,update_lowrank!rejection,r==0, and the no-Woodbury-QR guard. Plus LinearSolve-QR tests.Aqua.test_persistent_taskstimeout on loaded 1.12 hosts (the package spawns no load-time task — verified no@async/Threads.@spawn/Timer/__init__; the load subprocess exits in ~0.4s) is addressed by raising Aqua's wall-clocktmaxto 60s, with a comment explaining it relaxes an over-tight timeout, not a real defect.Runic-formatted. No new dependencies.
🤖 Generated with Claude Code
Co-Authored-By: Chris Rackauckas accounts@chrisrackauckas.com