Complete BLIS integration with reference LAPACK #666
Merged
Conversation
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
Test case:
```julia
using LinearSolve, blis_jll
A = rand(4, 4)
b = rand(4)
prob = LinearProblem(A, b)
sol = solve(prob,LinearSolve.BLISLUFactorization())
sol.u
```
throws:
```julia
julia> sol = solve(prob,LinearSolve.BLISLUFactorization())
ERROR: TypeError: in ccall: first argument not a pointer or valid constant expression, expected Ptr, got a value of type Tuple{Symbol, Ptr{Nothing}}
Stacktrace:
[1] getrf!(A::Matrix{Float64}; ipiv::Vector{Int64}, info::Base.RefValue{Int64}, check::Bool)
@ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:67
[2] getrf!
@ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:55 [inlined]
[3] #solve!SciML#9
@ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:222 [inlined]
[4] solve!
@ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:216 [inlined]
[5] #solve!SciML#6
@ LinearSolve ~/.julia/dev/LinearSolve/src/common.jl:209 [inlined]
[6] solve!
@ LinearSolve ~/.julia/dev/LinearSolve/src/common.jl:208 [inlined]
[7] #solve#5
@ LinearSolve ~/.julia/dev/LinearSolve/src/common.jl:205 [inlined]
[8] solve(::LinearProblem{…}, ::LinearSolve.BLISLUFactorization)
@ LinearSolve ~/.julia/dev/LinearSolve/src/common.jl:202
[9] top-level scope
@ REPL[8]:1
Some type information was truncated. Use `show(err)` to see complete types.
```
- Updated LinearSolveBLISExt to use both blis_jll and LAPACK_jll - Changed LAPACK function calls (getrf, getrs) to use liblapack instead of libblis - Added LAPACK_jll to weak dependencies and extension configuration - Created comprehensive test suite for BLIS + reference LAPACK functionality - Tests cover Float32/64, ComplexF32/64, accuracy, caching, and comparison with default solvers - All tests pass, confirming correct BLIS + reference LAPACK integration This fixes the issue where BLIS was incorrectly used for both BLAS and LAPACK operations. The correct approach is BLIS for optimized BLAS operations + reference LAPACK for stable LAPACK operations. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Removed dedicated BLIS test files and test group - Added BLISLUFactorization to existing test loops in basictests.jl - Added conditional loading of BLIS dependencies in tests - BLIS tests now run as part of standard "Concrete Factorizations" test suite - Tests are automatically skipped if BLIS dependencies are not available This follows the established pattern used by other factorization methods like MKL, making the integration cleaner and more maintainable. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
ChrisRackauckas-Claude
force-pushed
the
blis-reference-lapack
branch
from
bc67017 to
dc670a5
Compare
📊 BLIS Performance Benchmark ResultsI've created a comprehensive benchmark script to test the BLIS integration performance. Here are the results comparing BLIS with other LU factorization algorithms: Benchmark ScriptThe benchmark script and documentation are available in this gist: https://gist.github.com/ChrisRackauckas-Claude/18165706c1230defea412fa7e9e86d45 Performance Analysis
The benchmark uses the same detection logic as LinearSolve.jl itself, including proper platform detection for MKL, Apple Accelerate, etc. Results Plot |
📈 Benchmark Plot UploadThe performance comparison plot will be uploaded here showing BLIS vs OpenBLAS vs RecursiveFactorization performance across different matrix sizes. |
|
|
@ViralBShah there it is with reference LAPACK right there above. |
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.
Summary
This PR fixes the BLIS integration to properly use BLIS for BLAS operations and reference LAPACK for LAPACK operations, addressing the original issue where BLIS was incorrectly used for both.
Key Changes
getrf,getrs) fromlibblistoliblapackLAPACK_jllto weak dependencies and extension configurationLinearSolveBLISExt = ["blis_jll", "LAPACK_jll"]BLISLUFactorizationto standard test loops instead of separate test filesTechnical Implementation
Library Stack:
blis_jllLAPACK_jlllibblis, LAPACK functions useliblapackExtension Dependencies:
Testing
BLIS is now tested as part of the standard "Concrete Factorizations" test suite in
basictests.jl:Tests are automatically skipped if BLIS dependencies are not available, following the same pattern as other optional dependencies.
Performance Benefits
Before/After
Before:
After:
🤖 Generated with Claude Code