-
Notifications
You must be signed in to change notification settings - Fork 306
add support for iterative build of OpenBLAS with 64-bit integers #2753
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
add support for iterative build of OpenBLAS with 64-bit integers #2753
Conversation
|
I don't think adding _ilp64 suffix to the actual symbols is a good thing. Most codes I know of that does use ilp64 blas just assumes that the symbols are the same as usual. |
|
@akesandgren good point, I've switched back to using |
|
I agree it's simpler not to suffix the symbols, but otoh, numpy suggests using them to avoid clashes (https://numpy.org/devdocs/user/building.html#bit-blas-and-lapack), and SciPy says they actually need both (https://scipy.github.io/devdocs/dev/contributor/building.html#reference-for-build-options)... Could we build all three variants? |
|
@migueldiascosta so, two 64bit iterations: one iteration with |
|
overriding the symbols is (edit: noticed now that I misread the above, so nevermind) |
|
Conflict resolution needed. Also, I think we should try to get both the plain 32bit int and the 64_ symbol suffix functions into the same libopenblas.(a|so) library in case that isn't already part of this change. |
|
@jfgrimm the _64 suffix isn't supported by OpenBLAS yet (it uses its own implementation of cblas that is not in sync with reference lapack). It will take some time for the ecosystem to catch up (FlexiBLAS and BLIS also) the underscore is a little nasty as it was automatically added until this commit: the Debian approach is probably better, see |
|
Yes, I was just looking at the debian recipe. We'll want to fix the .pc file to match as well |
LIBNAMESUFFIX=64 auto-adds a _ in older OpenBLAS, no _ in newer. LIBPREFIX=libopenblas64 works around that like Debian (https://salsa.debian.org/science-team/openblas/-/blob/master/debian/rules?ref_type=heads)
|
done in |
Use LIBPREFIX and patch openblas64.pc
|
Test report by @jfgrimm Overview of tested easyconfigs (in order)
Build succeeded for 3 out of 4 (4 easyconfigs in total) |
|
Test report by @bartoldeman Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
Test report by @bartoldeman Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
Test report by @bartoldeman Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
@jfgrimm not sure why your 0.3.23 failed.. |
|
Could be some local hooks or something interfering... I'm on leave for a week starting today, so won't have an opportunity to check for a while. |
|
Test report by @bartoldeman Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
@jfgrimm any chance of rechecking your failed |
|
@bartoldeman will retry in a sec |
This comment was marked as outdated.
This comment was marked as outdated.
|
Test report by @jfgrimm Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
@bartoldeman success! also, fixed the minor merge conflict |
bartoldeman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
merging now that EB 5.1 has been released
…nblas add support for iterative build of OpenBLAS with 64-bit integers
(created using
eb --new-pr)