-
Notifications
You must be signed in to change notification settings - Fork 558
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
[Sundials 3.1] Build with OpenBLAS support #378
Conversation
If it works, we should merge it. Going forward we want to put the efforts into sundials 5.1, which seems to significantly address these sorts of issues. |
Note that the LAPACK will not work irrespective because sundials 3 does not support 64-bit ints for lapack, and the cmake will disable the lapack. |
I tested locally for a couple of platforms, with JuliaPackaging/BinaryBuilder.jl#604 the dependencies should be setup correctly. However right now the agents of the pipelines are all offline, so we don't the have the CI service. |
Should we move this to Sundials@3 to make way for Sundials@5 ? |
4e74c87
to
9fdf841
Compare
Should we proceed with this one? |
I need to finish JuliaPackaging/BinaryBuilder.jl#604 first. |
I suggest using https://github.com/LLNL/sundials/releases in this PR. As per the changes in the release, issues with LAPACK and KLU are resolved. |
@jd-lara Note that it is using the correct github repo, but we are building only release v3.1, which is the current compatible version for the current Sundials.jl. |
9fdf841
to
1f2e526
Compare
@ViralBShah After we fixed the issue in BinaryBuilder with setting up the correct dependencies, now OpenBlas should always be found in these builds. However, is it normal that none of the libraries is directly linked to libopenblas? E.g., libraries for
For macOS:
|
That is a bit odd. I wonder if this is only meaningful when lapack is enabled. @jd-lara ? @ChrisRackauckas? |
The makefile disables the linking if the BLAS is not a 32-bit BLAS. This is supposedly fixed in Sundials 5.1 |
I think we patch it, so it shouldn't be an issue (in theory). Anyways, we have sundials 3.1 as before - since BLAS is not getting linked, we have not much to gain by merging this. Best to update this PR for 5.1. |
I will make an attempt on 5.1 and possibly submit a PR if things work out ok. |
An update on 5.1.0 I am currently trying to get the CMAKE to pass everything. Current status:
About the Fortran name mangling. The code is here https://github.com/LLNL/sundials/blob/master/config/SundialsFortran.cmake and I implemented the same changes as in the patch to see if it works but I still get the same result. Determining the proper name mangling scheme in fortran is required for LAPACK to work. Any ideas? In the new version of Sundials, LAPACK compatibility is checked in the file |
see LLNL/sundials#23 for updates on the build of Sundials 5.1 |
@giordano The reason why none of the libraries are linked against lapack is that we need a new library product - The tests with sundials 5.1 are all passing with lapack (#460). |
I've disabled blas and lapack here. This should become a stable build for sundials 3.1. |
This should make it possible to build with support for OpenBLAS, but it's not clear to me if it's still desired. CC: @ViralBShah.
Note: it's a draft because this requires JuliaPackaging/BinaryBuilder.jl#604.