-
-
Notifications
You must be signed in to change notification settings - Fork 76
Fix default algorithm for sparse CUDA matrices to LUFactorization #828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix default algorithm for sparse CUDA matrices to LUFactorization #828
Conversation
Previously: - CuSparseMatrixCSC defaulted to GenericLU (not GPU compatible) - CuSparseMatrixCSR fell back to Krylov when CUDSS unavailable (not GPU compatible) - Symmetric sparse matrices defaulted to Cholesky (not GPU compatible) Changes: - Both CuSparseMatrixCSC and CuSparseMatrixCSR now default to LUFactorization - Removed Krylov fallback - if CUDSS is not loaded, clear error message is shown - Added cudss_loaded() support for CuSparseMatrixCSC - Added error_no_cudss_lu() for CuSparseMatrixCSC Fixes SciML#827 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implementation DetailsCode Changes1. ext/LinearSolveCUDAExt.jlSimplified function LinearSolve.defaultalg(A::CUDA.CUSPARSE.CuSparseMatrixCSR{Tv, Ti}, b,
assump::OperatorAssumptions{Bool}) where {Tv, Ti}
LinearSolve.DefaultLinearSolver(LinearSolve.DefaultAlgorithmChoice.LUFactorization)
end
Added function LinearSolve.defaultalg(A::CUDA.CUSPARSE.CuSparseMatrixCSC{Tv, Ti}, b,
assump::OperatorAssumptions{Bool}) where {Tv, Ti}
LinearSolve.DefaultLinearSolver(LinearSolve.DefaultAlgorithmChoice.LUFactorization)
end
Added function LinearSolve.error_no_cudss_lu(A::CUDA.CUSPARSE.CuSparseMatrixCSC)
if !LinearSolve.cudss_loaded(A)
error("CUDSS.jl is required for LU Factorizations on CuSparseMatrixCSC. Please load this library.")
end
nothing
end
2. ext/LinearSolveCUDSSExt.jlAdded LinearSolve.cudss_loaded(A::CUDSS.CUDA.CUSPARSE.CuSparseMatrixCSC) = true
Design RationaleThe previous approach of falling back to Krylov methods when CUDSS was unavailable didn't work because Krylov methods also suffer from scalar indexing issues on GPU. The new approach is simpler and more user-friendly:
This matches the philosophy that it's better to give a clear error message than to silently fall back to an algorithm that will fail anyway. Files Modified
|
Development ProcessSteps Taken
Next Steps
CI StatusCI tests are currently running. The PR includes changes to CUDA extension files, so CUDA-specific tests will be particularly important to monitor. |
Updated defaultalg() to check if CUDSS is loaded and error immediately if not available, rather than removing the check entirely or falling back to Krylov (which also doesn't work on GPU). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Update: Added cudss_loaded() CheckUpdated the implementation based on feedback. Now function LinearSolve.defaultalg(A::CUDA.CUSPARSE.CuSparseMatrixCSR{Tv, Ti}, b,
assump::OperatorAssumptions{Bool}) where {Tv, Ti}
if LinearSolve.cudss_loaded(A)
LinearSolve.DefaultLinearSolver(LinearSolve.DefaultAlgorithmChoice.LUFactorization)
else
error("CUDSS.jl is required for LU Factorizations on CuSparseMatrixCSR. Please load this library.")
end
endSame logic applies to Benefits:
|
Summary
This PR fixes issue #827 by making
LUFactorizationthe default algorithm for bothCuSparseMatrixCSCandCuSparseMatrixCSR. Previously, these sparse CUDA matrix types defaulted to algorithms that are not GPU-compatible and cause scalar indexing errors.Problem
Currently, the default algorithm choice for sparse CUDA matrices has several issues:
CuSparseMatrixCSC: Always defaults toGenericLUwhich is not GPU compatible (causes scalar indexing errors)CuSparseMatrixCSRwithout CUDSS: Falls back toKrylovJL_GMRESwhen CUDSS is not loaded, which is also not GPU compatible (causes scalar indexing errors)All of these issues result in poor user experience with confusing scalar indexing errors.
Solution
This PR makes the following changes:
1. LinearSolveCUDAExt.jl
defaultalg()forCuSparseMatrixCSR: Now always returnsLUFactorizationinstead of falling back to Krylov methodsdefaultalg()forCuSparseMatrixCSC: New method that returnsLUFactorizationerror_no_cudss_lu()forCuSparseMatrixCSC: Provides clear error message when CUDSS is required but not loaded2. LinearSolveCUDSSExt.jl
cudss_loaded()forCuSparseMatrixCSC: Returnstruewhen CUDSS is available for CSC matricesBenefits
Testing
The package loads successfully without errors. Full CUDA tests require CUDA hardware and the CUDSS package.
Fixes
Fixes #827
🤖 Generated with Claude Code