Skip to content
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

WIP: Wrap BLIS #431

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

WIP: Wrap BLIS #431

wants to merge 2 commits into from

Conversation

ChrisRackauckas
Copy link
Member

Test case:

using LinearSolve, blis_jll

A = rand(4, 4)
b = rand(4)
prob = LinearProblem(A, b)
sol = solve(prob,LinearSolve.BLISLUFactorization())
sol.u

throws:

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!#9
   @ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:222 [inlined]
 [4] solve!
   @ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:216 [inlined]
 [5] #solve!#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.

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!#9
   @ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:222 [inlined]
 [4] solve!
   @ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:216 [inlined]
 [5] #solve!#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.
```
@ChrisRackauckas
Copy link
Member Author

@staticfloat do you know why it would give that error? That's the same way I'm loading the other BLAS's. I thought this would be a nice factorization choice for you to give a try on EPYC.

Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Merging #431 (afcc28e) into main (a455e27) will decrease coverage by 37.63%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##             main     #431       +/-   ##
===========================================
- Coverage   63.80%   26.17%   -37.63%     
===========================================
  Files          26       27        +1     
  Lines        2097     2204      +107     
===========================================
- Hits         1338      577      -761     
- Misses        759     1627      +868     
Files Coverage Δ
src/extension_algs.jl 66.66% <ø> (-2.90%) ⬇️
ext/LinearSolveBLISExt.jl 0.00% <0.00%> (ø)

... and 16 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@blasfunc, chkargsok
using LinearSolve: ArrayInterface, BLISLUFactorization, @get_cacheval, LinearCache, SciMLBase

const global libblis = dlopen(blis_jll.blis_path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unsafe; it's telling Julia to dlopen() a file at compile-time and save the pointer to that file into the precompile cache. You should just use the bljs variable from blis_jll in the first place; it will do the dlopen() for you at the appropriate time (e.g. in __init__()).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

julia> blis_jll.bljs
ERROR: UndefVarError: `bljs` not defined
Stacktrace:
 [1] getproperty(x::Module, f::Symbol)
   @ Base ./Base.jl:31
 [2] top-level scope
   @ REPL[18]:1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, typo; blis

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ERROR: could not load symbol "dgetrf_":
dlsym(0x8f1f2a90, dgetrf_): symbol not found
Stacktrace:
 [1] getrf!(A::Matrix{Float64}; ipiv::Vector{Int32}, info::Base.RefValue{Int32}, check::Bool)
   @ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:67
 [2] getrf!
   @ ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:55 [inlined]
 [3] solve!(cache::LinearSolve.LinearCache{…}, alg::LinearSolve.BLISLUFactorization; kwargs::@Kwargs{})
   @ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:224
 [4] solve!
   @ LinearSolveBLISExt ~/.julia/dev/LinearSolve/ext/LinearSolveBLISExt.jl:218 [inlined]
 [5] #solve!#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
   @ ~/Desktop/test.jl:29
Some type information was truncated. Use `show(err)` to see complete types.

Also dgetrf_32 and dgetrf_64 give the same.

@ChrisRackauckas
Copy link
Member Author

I guess my issue is that BLIS isn't a full BLAS and this is in FLAME?

@amontoison
Copy link
Contributor

amontoison commented Nov 12, 2023

getrf! is a LAPACK routine. BLIS is an implementation BLAS so you need to combine blis_jll.jl with LAPACK_jll.jl. It's what is done in AppleAccelerate.jl.

We should probably update BLISBLAS.jl to do that automatically like MKL.jl or AppleAccelerate.jl.

@jd-foster
Copy link

@amontoison I've taken up your idea and shown it can be done in #498, but I also think the best course would be to enable this in one go in a package automatically (i.e. in BLISBLAS.jl and use that in the extension.)

@amontoison
Copy link
Contributor

@jd-foster
I opened a PR in BLISBLAS.jl for the 32-bit integer version that is used by many artifact package (such as Ipopt, MUMPS, ...) and the PR was never merged.

I don't know if BLISBLAS.jl is still maintained.

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.

4 participants