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

BUG: Fix cblas detection for the wheel builds #24222

Merged
merged 1 commit into from Jul 20, 2023

Conversation

seberg
Copy link
Member

@seberg seberg commented Jul 20, 2023

This is a mess and doesn't attempt to fix the issues around gh-24200, but it should fix the issue of our wheels not using CBLAS.

The suffix may be directly injected into the flags to get the define and there is also a tangle between HAVE_CBLAS and HAVE_LAPACK and we only probe the first.

I tried this locally by checking that if the define exist redefining it to nonesense gives the slow version.
(I cannot test the full thing without a prefixed blas locally, because setting the prefix will not avoid blas use in LAPACK meaning you don't use CBLAS, but the blas symbols used by lapack are missing)


CC @mattip and @rgommers, I do not plan to attempt the "full" fix, it seems that may require untangling the way these things are set. I think this works, but we may have to give it a roll to be sure probably.

This is a mess and doesn't attempt to fix the issues around numpygh-24200, but
it should fix the issue of our wheels not using CBLAS.

The suffix may be directly injected into the flags to get the define and
there is also a tangle between `HAVE_CBLAS` and `HAVE_LAPACK` and we only
probe the first.

I tried this locally by checking that *if* the define exist redefining
it to nonesense gives the slow version.
(I cannot test the full thing without a prefixed blas locally, because
setting the prefix will not avoid blas use in LAPACK meaning you don't
use CBLAS, but the blas symbols used by lapack are missing)
@seberg
Copy link
Member Author

seberg commented Jul 20, 2023

OK, I downloaded and installed the artifact wheel locally and it seems fine.

@rgommers
Copy link
Member

Thanks @seberg, that seems to do the job. At least I spot checked an x86-64 manylinux and an arm64 macOS wheel build, and both logs show:

Run-time dependency openblas found: YES 0.3.23.dev
Checking if "CBLAS" with dependency OpenBLAS: links: YES

This is a mess and doesn't attempt to fix the issues around gh-24200, but it should fix the issue of our wheels not using CBLAS.

Agreed it is messy. Longer term these checks should live in Meson and not here, so we can simply write something like:

openblas_dep = dependency('openblas',
  version : '>=0.3.21',
  language: 'c',
  modules: [
    'interface: ilp64',
    'symbol-suffix: 64_',
    'cblas',
  ]
)
# Query properties as needed:
has_cblas = openblas.get_variable('cblas')
is_ilp64 = openblas_dep.get_variable('interface') == 'ilp64'
blas_symbol_suffix = openblas_dep.get_variable('symbol-suffix')

and only express our actual build/dependency logic in our own meson.build files.

I'll need to finish up mesonbuild/meson#10921 within the next couple of months for that.

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Jul 20, 2023
@charris
Copy link
Member

charris commented Jul 20, 2023

Whoa, I added a label and the wheel builds restarted. Might be something a bit off in the triggering.

@charris charris merged commit 570b209 into numpy:main Jul 20, 2023
95 checks passed
@charris
Copy link
Member

charris commented Jul 20, 2023

Thanks Sebastian.

@seberg seberg deleted the fix-cblas-wheels branch July 20, 2023 22:17
@seberg seberg added this to the 1.26.0 release milestone Jul 21, 2023
@seberg seberg added the Meson Items related to the introduction of Meson as the new build system for NumPy label Jul 21, 2023
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Aug 3, 2023
@charris charris removed this from the 1.26.0 release milestone Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
00 - Bug 36 - Build Build related PR Meson Items related to the introduction of Meson as the new build system for NumPy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants