-
Notifications
You must be signed in to change notification settings - Fork 18
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
Use LBT to forward BLAS and LAPACK calls to Accelerate #58
Conversation
This will naturally fail CI on any macOS older than 13.3 Anecdotally, Accelerate on my M1 Pro runs the LinearAlgebra test suite pretty quickly:
Versus OpenBLAS:
Although I do see that we run slightly more tests on OpenBLAS; not sure why that is. |
As an update, macOS v13.4 beta 3 fixes the dsptrf bug; running the LinearAlgebra test suite with only Accelerate loaded (no external LAPACK) passes! |
Wow that's quick. I suppose in that case the simplest thing is to make macOS 13.4 the min version and then remove all the LAPACK overlay stuff. |
I am trying to run the ILP64 accelerate branch on MacOS 13.3.1 (on an M2 chip). I get an error when LBT tries to load lapack from the LAPACK_jll artifact. The error I get is:
This seems to indicate that there was an error in the autodetect_interface function in LBT that tries to determine if it's a 32 or 64 bit library. I've tried Any help would be appreciated. I am not on the 13.4 beta with the fix for dsptrf so my understanding is that I need to use this external LAPACK lib with Accelerate BLAS This on the head of sf/ilp64_accelerate, commit d05a891 |
@Moblin88 This works for me. I just pushed an update for LAPACK 3.11 as well, and made that the minimum. Can you try it out? |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #58 +/- ##
==========================================
+ Coverage 80.26% 82.81% +2.54%
==========================================
Files 4 4
Lines 152 192 +40
==========================================
+ Hits 122 159 +37
- Misses 30 33 +3
☔ View full report in Codecov by Sentry. |
@staticfloat I have reinstated the earlier capabilities in this package and would like to merge this PR, if it looks good to you. The DSP and Array functions do not bring additional package dependencies, so are perhaps ok to leave here for now. We can refactor this into more packages later, but removing the code felt like we would forget about it. It works fine and passes tests, and hopefully will help others build further. |
729a176
to
94d4c0a
Compare
Introduce the use of LBT to transparently use Accelerate for BLAS and LAPACK operations. This re-architecting causes Accelerate to pass the full LinearAlgebra test suite (thanks to the usage of an external LAPACK_jll to paper over bugs in `dsptrf()`; hopefully no longer necessary in a future macOS update).
Tell the user what version they're running if it fails
Use LAPACK 3.11 Run tests only on Apple Set compat to Julia 1.9 Add Statistics and DSP for tests Re-enable Windows tests to make sure that AppleAccelerate loads without error and is a no-op
94d4c0a
to
e3753ce
Compare
It's working for me now on the master branch that was just merged with LAPACK 3.11.0. It's also WAYY faster to multiply large dense matrices! |
We will be able to remove the LAPACK dependency once macos 13.4 is out. |
Is it possible to do a new release of AppleAccelerate.jl? |
My preference is to wait for macos 13.4 and remove the lapack dependency and then make a release. Would you prefer sooner? |
No that's fine. I just wanted to add a comment about AppleAccelerate.jl in the documentation of JuliaHSL and explained that |
This throws away most of the previous version, instead opting to re-architect this package to make use of LBT to transparently use Accelerate for BLAS and LAPACK operations. Further enhancements to re-introduce the DSP functionality can be made, potentially in a separate package if we want to keep this one lightweight, as it may end up at the bottom of many dependency trees.
This re-architecting causes Accelerate to pass the full LinearAlgebra test suite (thanks to the usage of an external LAPACK_jll to paper over bugs in
dsptrf()
; hopefully no longer necessary in a future macOS update).Fixes #45