Fix GPU Cholesky cache initialization for non-square matrices - #857
Fix GPU Cholesky cache initialization for non-square matrices#857ChrisRackauckas-Claude wants to merge 4 commits into
Conversation
When using DefaultLinearSolver with non-square GPU matrices (e.g., for least squares problems), the init_cacheval function for CholeskyFactorization would fail because it tried to compute cholesky(A) on a non-square matrix. The fix checks assumptions.issq before attempting Cholesky factorization and returns nothing for non-square matrices, allowing the DefaultLinearSolver to properly use QRFactorization instead. Fixes SciML/NonlinearSolve.jl#746 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
91c9aec to
854d90f
Compare
Add check for assumptions.issq in the SparseArrays extension's NormalCholeskyFactorization init_cacheval to avoid DimensionMismatch errors when DefaultLinearSolver initializes caches for non-square GPU matrices. This is part of the fix for SciML/NonlinearSolve.jl#746 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Added a second fix for the The previous commit fixed This commit adds the same |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CI Status UpdateThe key downstream tests are passing:
The failing tests appear to be pre-existing issues:
GPU tests run on Buildkite separately and will validate the fix for non-square GPU matrices. |
The previous change incorrectly returned nothing for ALL non-square matrices (sparse and GPU), but NormalCholeskyFactorization actually works with non-square matrices because it factors A'*A which is square. The issue was only with GPU arrays where ArrayInterface.cholesky_instance fails for non-square. Sparse CPU arrays handle this correctly. Now the check only applies to GPUArraysCore.AnyGPUArray, allowing sparse CPU matrices to use the original cholesky_instance path. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fix for Non-Square Tests FailureThe previous change incorrectly returned The issue was only with GPU arrays where Fix: Changed the condition to only apply to # Before (incorrect - broke sparse CPU):
elseif !assumptions.issq
nothing
# After (correct - only affects GPU):
elseif A isa LinearSolve.GPUArraysCore.AnyGPUArray && !assumptions.issq
nothing |
✅ All Core Tests PassingAfter the fix to only apply the non-square check to GPU arrays (not sparse CPU arrays), all Core tests are now passing:
The only remaining failures are pre-existing infrastructure issues:
Summary of changes:
The fix allows |
|
@AJ0070 look into rebasing this? |
`solve(LinearProblem(A, b))` with a non-square GPU `A` threw
`DimensionMismatch: matrix is not square`, in two different places.
Cache initialization. `_init_default_cacheval` builds a cacheval for every
algorithm slot before it knows which one it will use, and two of those
slots call `cholesky` on `A` itself:
- `CholeskyFactorization` on `AnyGPUArray`, which calls
`cholesky(A; check = false)`.
- `NormalCholeskyFactorization` on `AnyGPUArray`, which calls
`ArrayInterface.cholesky_instance(A)`. That algorithm factorizes `A'A`,
so a non-square `A` is a supported least-squares input, and the generic
method already sidesteps the shape by instancing an empty `Symmetric`.
Only the GPU method built the instance from `A`. The guard is scoped to
GPU input: `cholesky_instance` handles a non-square sparse `A`, and
`solve!` stores a real factorization into that slot afterwards.
The wide solve. Past cache initialization, a wide `A` then failed inside
`QRFactorization`: a GPU `qr` factors it, but solving with the result
builds `UpperTriangular(R)` on an `R` that is not square. `MinNormQR`
already exists for exactly this, used by the BandedMatrices and
FastAlmostBandedMatrices extensions, so route wide GPU input through `Aᵀ`
the same way. That also makes the answer the minimum-norm one, matching
dense `\` on the CPU.
Measured on a T4, against main:
before tall 4x2 FAIL DimensionMismatch (4, 2)
wide 2x4 FAIL DimensionMismatch (2, 4)
after tall 4x2 OK relerr=2.47e-6
wide 2x4 OK relerr=5.62e-7 ‖x‖ 0.209165 vs 0.209165
square 3x3 OK relerr=6.91e-8
The wide test asserts `‖x‖` against LAPACK's, not just the residual:
agreeing on the residual alone would not distinguish the minimum-norm
solution from any other point on the solution manifold.
Revives SciML#857, whose guard covered only the `CholeskyFactorization` slot.
The `NormalCholeskyFactorization` slot moved into `sparsearrays.jl` in the
eight months since, and fails first, so that fix alone no longer resolves
the reported failure; verified on a T4 that it does not.
Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
* Solve a non-square GPU system instead of throwing
`solve(LinearProblem(A, b))` with a non-square GPU `A` threw
`DimensionMismatch: matrix is not square`, in two different places.
Cache initialization. `_init_default_cacheval` builds a cacheval for every
algorithm slot before it knows which one it will use, and two of those
slots call `cholesky` on `A` itself:
- `CholeskyFactorization` on `AnyGPUArray`, which calls
`cholesky(A; check = false)`.
- `NormalCholeskyFactorization` on `AnyGPUArray`, which calls
`ArrayInterface.cholesky_instance(A)`. That algorithm factorizes `A'A`,
so a non-square `A` is a supported least-squares input, and the generic
method already sidesteps the shape by instancing an empty `Symmetric`.
Only the GPU method built the instance from `A`. The guard is scoped to
GPU input: `cholesky_instance` handles a non-square sparse `A`, and
`solve!` stores a real factorization into that slot afterwards.
The wide solve. Past cache initialization, a wide `A` then failed inside
`QRFactorization`: a GPU `qr` factors it, but solving with the result
builds `UpperTriangular(R)` on an `R` that is not square. `MinNormQR`
already exists for exactly this, used by the BandedMatrices and
FastAlmostBandedMatrices extensions, so route wide GPU input through `Aᵀ`
the same way. That also makes the answer the minimum-norm one, matching
dense `\` on the CPU.
Measured on a T4, against main:
before tall 4x2 FAIL DimensionMismatch (4, 2)
wide 2x4 FAIL DimensionMismatch (2, 4)
after tall 4x2 OK relerr=2.47e-6
wide 2x4 OK relerr=5.62e-7 ‖x‖ 0.209165 vs 0.209165
square 3x3 OK relerr=6.91e-8
The wide test asserts `‖x‖` against LAPACK's, not just the residual:
agreeing on the residual alone would not distinguish the minimum-norm
solution from any other point on the solution manifold.
Revives #857, whose guard covered only the `CholeskyFactorization` slot.
The `NormalCholeskyFactorization` slot moved into `sparsearrays.jl` in the
eight months since, and fails first, so that fix alone no longer resolves
the reported failure; verified on a T4 that it does not.
Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
* Instance an empty Cholesky for a non-square GPU A instead of returning nothing
* Instance an empty Cholesky in the NormalCholesky GPU slot too
* Compare the QR solve on the host, not against a device array
---------
Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
|
Closing as superseded: 679 commits behind master; the change is either on master already or no longer applies. 🤖 Posted by an AI agent — harness: Claude Code, model: claude-opus-5[1m]. Chris asked for these closures directly. |
Summary
Fixes SciML/NonlinearSolve.jl#746
When using
DefaultLinearSolverwith non-square GPU matrices (e.g., for least squares problems in NonlinearSolve.jl), theinit_cachevalfunction forCholeskyFactorizationwould fail withDimensionMismatch: matrix is not squarebecause it tried to computecholesky(A)on a non-square matrix.Root Cause
The
@generatedinit_cachevalforDefaultLinearSolverinitializes caches for all possible algorithms upfront, includingCholeskyFactorization. For non-square GPU matrices, this fails even thoughCholeskyFactorizationwould never actually be used (thedefaultalgfunction correctly selectsQRFactorizationfor non-square matrices).Fix
The fix checks
assumptions.issqbefore attempting Cholesky factorization ininit_cacheval(::CholeskyFactorization, ::GPUArraysCore.AnyGPUArray, ...)and returnsnothingfor non-square matrices, allowing the cache initialization to succeed.Changes
src/factorization.jl: Added square matrix check ininit_cachevalforCholeskyFactorizationwith GPU arraystest/gpu/cuda.jl: Added tests for non-square GPU matrices (overdetermined and underdetermined systems)Test plan
🤖 Generated with Claude Code