-
-
Notifications
You must be signed in to change notification settings - Fork 35
Add low-level wrappers for BLAS and LAPACK #1403
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
base: master
Are you sure you want to change the base?
Conversation
|
The |
|
Hello @ViralBShah, I autogenerated them from a custom Julia script that directly parse the Fortran files in Reference-LAPACK. Note that that I will visit @langou next month during ten days and we plan to work on that. |
642d561 to
b71c688
Compare
|
There are a few lapack calls with one argument that need fixing too. Perhaps the generator script can be updated? It would be nice to have a In theory, we can use Clang.jl to automatically generate the entire API for CBLAS. |
|
@ViralBShah I can upload my Fortran parser of F77 but I need to clean it a little bit. What would be great to have the "buffer" version of the LAPACK routines with LAPACKE. |
3ad63b5 to
3db0f47
Compare
|
The reason to do cblas would be then to wrap that into the low level Julia interface and have Linear Algebra use that and delete the fortran bindings altogether. The LAPACKE style overhaul can easily be done in a new external package. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1403 +/- ##
===========================================
- Coverage 93.89% 78.07% -15.82%
===========================================
Files 34 36 +2
Lines 15893 20249 +4356
===========================================
+ Hits 14922 15809 +887
- Misses 971 4440 +3469 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
We'll also need to see the impact this has on system image size. |
|
All wrappers are removed in |
|
The tests are all passing now. I will check the system size impact. Hopefully it isn’t much. Given that it is all working now, perhaps not much incentive to change to cblas now, but this puts us in a really good position to do it whenever we need to. |
d8aeece to
db9d9ca
Compare
|
These methods are being generated twice. Should we just edit these by hand or would you like to regenerate the files? |
|
This PR adds 5MB to the system image on Linux. This seems acceptable to me. My hope is that we can separate out the bits for |
|
Is it possible for the wrapper generator script to annotate the types for all the function calls? Currently these are all |
|
Hello @ViralBShah, in general we don't type the arguments of the wrappers because it is low-level routines for advance users. I can add types but I am scared to be too much restrictive. With @langou, we discussed about the Julia wrappers and he thinks that we should interface CBLAS / LAPACKE instead in LinearAlgebra. Viral, if you want to meet, we are available this week with Julien. We are working together this week on reference-LAPACK. |
|
I am happy to meet, althoughI do not have much more to contribute than what I already have. I was wondering myself if we should use the C interfaces and auto generate. I think it should be ok to add types to the methods and widen them for things like Integer. The rest have very little room and will be enforced by ccall anyways. Perhaps we get this for free in Clang.jl. |
|
It seems like Apple Accelerate does not use LAPACKE. |
|
The other question is LBT support. However it should be easy to add if C API support is missing. Perhaps worth considering doing this in an external package? |
|
If we have these, shouldn't they be called Who is this PR for, exactly? |
|
People generally keep asking for a way to call the entire LAPACK and BLAS from Julia - not just our prepackaged factorizations. This adds the full API surface. While it is definitely useful, it is worthwhile to discuss where it should live. |
|
In my mind, exposing a bunch of unsafe functions like this is not much better than telling them to use ccall. In both cases you need to look up a bunch of low-level arguments and understand low-level types etc. or it will crash. One alternative would just be to add a |
|
The complaint of @langou when he tried Julia is that he doesn’t understand why some arguments are exposed and others are not, and I agree with him. The design of the routines / high-level interfaces in What would be nice is to expose documented high-level wrappers without the prefixes If we want something with very limited arguments, then we interface them in high-level Julia functions like Another possible approach could be to keep the old methods (in This would "just" involve duplicating some methods like |
|
@amontoison How do you feel about a separate package? That package could even use Preferences to pick a BLAS and LAPACK and do other nice things. |
That's a good argument for adding more high-level methods of these functions that expose more arguments, I agree. I'm not sure what it has to do with the present PR, which is not much better than doing your own |
Picking up on this from the LBT perspective, LBT will forward the CBLAS and LAPACKE functions to the library only if it provides them. The exception to that is a library that contains both LP64 & ILP64 symbols in Fortran but only LP64 symbols in CBLAS - then LBT will use internal forwards for the ILP64 CBLAS functions to the library's ILP64 Fortran interface. We don't in general have a bridge layer for the CBLAS symbols, or for any LAPACKE symbols, in LBT. We could extend the CBLAS bridge creation to be a more general system that would use LBT bridge functions to go C->Fortran if CBLAS symbols are not detected, and a similar thing for LAPACKE as well. One question is, how complete is the interface layer for CBLAS and LAPACKE in the "reference" version? I do see a few issues open where people say a LAPACK function is missing in LAPACKE (Reference-LAPACK/lapack#962, OpenMathLib/OpenBLAS#5523) Edit: Although looking at the LAPACKE functions some more, they aren't just simple wrappers around the LAPACK functions I guess. I don't know if we really would want to bring an entire fallback system into LBT that isn't just a simple wrapper. |
|
We really should create an external package for this capability which will be way quicker to iterate on and improve. |
CBLAS should be fully covering BLAS. I do not know how much LAPACKE is covering LAPACK. We have added wrappers to LAPACKE organically based on users' demands. The rationale has been that internal auxiliary LAPACK routines do not need LAPACKE wrappers. So we made an initial guess when we released LAPACKE. We only LAPACKE-ed the LAPACK routines labelled as "drivers". And then as people needed deeper routines we added these in LAPACKE based on request. Some takeaways: • LAPACKE does not cover the whole LAPACK, • But it covers what matters most, • If something is missing that seems relevant, we would consider adding it to LAPACKE.
This is absolutely correct. In addition of being a C wrapper to LAPACK, LAPACKE has features such as () column major / row major, () automatic memory allocation, (*) NaN checking. |
|
Thinking about the CBLAS and LAPACKE bits a bit more, I think what would be the best way of doing it would be building CBLAS-only and LAPACKE-only libraries in Yggdrasil (based on the Reference-LAPACK repo) that link against LBT, and then at the Julia level we can load those libraries if needed (e.g., we have a BLAS/LAPACK library that doesn't provide them). That can keep LBT light-weight still, and allow us to not need to duplicate the LAPACKE/CBLAS wrappers. I think that would still need a small change in LBT, where we add a CBLAS and a LAPACKE symbol to the automatic detection logic to identify suffixes, although I don't think we can reliably do integer autodetect for things like CBLAS because it would call BLAS, so we would need the BLAS library loaded already. |
Superseed #1386
cc @ViralBShah @langou