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

Improve BLAS/LAPACK support: dependency detection, UX, and ILP64 #17244

Open
2 of 17 tasks
rgommers opened this issue Oct 17, 2022 · 15 comments
Open
2 of 17 tasks

Improve BLAS/LAPACK support: dependency detection, UX, and ILP64 #17244

rgommers opened this issue Oct 17, 2022 · 15 comments
Labels
Meson Items related to the introduction of Meson as the new build system for SciPy

Comments

@rgommers
Copy link
Member

rgommers commented Oct 17, 2022

This is an issue to keep track of and break down and plan the work for making the BLAS and LAPACK dependency detection as robust and easy to use as possible.

The related items in gh-16293 are:

  • Improved BLAS/LAPACK detection/support (it works now, but only using pkg-config): Feature: MKL dependency mesonbuild/meson#2835
  • ILP64 support. This is now still tested in one numpy.distutils based CI job, and not supported yet in the Meson-based build.

Let's break this down more. Here is what I think we need:

  • detection of any supported BLAS/LAPACK library via pkg-config
  • detection of any supported BLAS/LAPACK library installed in standard paths (for native, i.e. now cross-compilation, builds) automatically via Meson
  • detection of any supported BLAS/LAPACK library via CMake as a fallback, if that is installed, with the same name as for pkg-config
  • ability to specify which BLAS/LAPACK library to use plus the directory in which it is installed in a native-file.ini or cross-file.ini.
  • Work around potential issue with OpenBLAS not including LAPACK symbols: BUG: Meson build creates broken scipy.sparse.linalg._isolve on archlinux w/ system openblas #17465
  • Add support for Flexiblas #17362
  • passing through selections via pip/build instead of having to invoke meson setup separately: Passing through project-specific config options mesonbuild/meson-python#54
  • ability to specify a preferred order (ala NPY_LAPACK_ORDER) in an ini file
  • comprehensive documentation on all of these methods
  • ILP64 support in the Meson build
    • implement support in Meson
    • move SciPy's CI job to Meson
    • user interface support so we can select ILP64 via pip/build/ini-file
  • (and to enable work on this) an easy way of testing all this, locally and in CI (perhaps in conda-forge, ala TEST: 1.8.x + blas variants conda-forge/scipy-feedstock#199 - but first need the ability to just run the configure stages, rather than a heavy build for a large test matrix)). Tests should be able to select one or all of:
    • library: OpenBLAS, MKL, Netlib
    • interface: LP64 and ILP64 flavors
    • detection method: pkg-config, CMake, Meson system, ini file
  • add a CI job using MSVC + Intel Fortran + MKL: WIP: CI: add a job with Intel Fortran + MSVC in Azure CI #16957

A bunch of these things are in progress already at mesonbuild/meson#10921.

@rgommers rgommers added the Meson Items related to the introduction of Meson as the new build system for SciPy label Oct 17, 2022
@Enchufa2
Copy link

Enchufa2 commented Nov 6, 2022

As I said here, adding 'BLAS' to the list of dependencies makes meson trigger FindBLAS, i.e.:

blas = dependency(blas_name, 'BLAS')
lapack = dependency(lapack_name, 'LAPACK')

So this covers the point of detecting "any supported BLAS/LAPACK library via CMake as a fallback".

@rgommers
Copy link
Member Author

rgommers commented Nov 7, 2022

@Enchufa2 thanks for testing that on Fedora and suggesting this improvement. I believe it works with Flexiblas - and with Netlib BLAS and ATLAS. But it will not work with at least MKL and Accelerate. MKL uses the g77 ABI rather than gfortran one, so it will lead to segfaults if we're blindly building against it without enabling our ABI wrappers for cdotc & co. Accelerate is completely unsupported, so we need to reject it.

I wish it was as simple as adding the above two-liner to add support for any BLAS library across platforms and packaging systems, but I'm pretty sure we cannot get away with that.

EDIT: to clarify - it will work with "MKL via Flexiblas" (because Flexiblas has basically the same ABI wrappers as we have), but it will not work with MKL directly. And when just saying "give me any BLAS" we have no guarantee that we won't get MKL directly. And I'd say that getting MKL directly is a lot more likely than getting Flexiblas.

@Enchufa2
Copy link

Enchufa2 commented Nov 7, 2022

But the method is not really blind, is it? Unless I'm missing something, you are already checking the name of the one detected here:

uses_mkl = blas.name().to_lower().startswith('mkl') or lapack.name().to_lower().startswith('mkl')

@rgommers
Copy link
Member Author

rgommers commented Nov 7, 2022

But the method is not really blind, is it?

I'd need to test, but I expect that if Meson asks CMake for 'BLAS' then the value for .name() will be BLAS rather than mkl. A quick check with this seems to confirm:

# install CMake and MKL in a conda env
blas = dependency('BLAS')
error(blas.name())

Run-time dependency blas found: YES
scipy/meson.build:126:0: ERROR: Problem encountered: BLAS

To verify we're actually using CMake, remove only cmake from the env, and see that we get:

scipy/meson.build:124:0: ERROR: Dependency "BLAS" not found, tried pkgconfig

I think it'll require custom support (like what I'm in the process of implementing) in Meson for this to return the name of the particular BLAS library that CMake detects.

@Enchufa2
Copy link

Enchufa2 commented Nov 7, 2022

Then this is just a lack of support in Meson, because FindBLAS does return all the information about the library found, so it should be straightforward to expose it.

@rgommers
Copy link
Member Author

rgommers commented Nov 7, 2022

Then this is just a lack of support in Meson, because FindBLAS does return all the information about the library found, so it should be straightforward to expose it.

I suspect it may be by design that the name is the one you asked for. Not many dependencies work like BLAS, which is kind of a virtual dependency which can have multiple implementations. I think it'll require a BLASCMakeDependency, similar to the OpenBLASCMakeDependency I'm adding in mesonbuild/meson#10921. I'll try to make this work.

@eli-schwartz
Copy link
Contributor

Then this is just a lack of support in Meson, because FindBLAS does return all the information about the library found, so it should be straightforward to expose it.

I'm reading /usr/share/cmake/Modules/FindBLAS.cmake and I don't see anywhere whatsoever that it returns that information.

It provides a variable describing the list of library filenames to link to -- it does not tell you anything about which ${BLA_VENDOR} was selected if it defaulted to searching for whatever is available.

@Enchufa2
Copy link

Enchufa2 commented Nov 8, 2022

It gets you the linker flags without the -l.

@eli-schwartz
Copy link
Contributor

So what do you do with that information? Hand-rolled regex matches against linker flags inside CMakeLists.txt?

@SomeoneSerge
Copy link

SomeoneSerge commented May 2, 2023

passing through selections via pip/build instead of having to invoke meson setup separately

Is there any reason not to run meson setup directly though? 🙃 I'd expect this to be much less fragile and more explicit than hiding meson behind pip. Meson also provides a very familiar interface for choosing details like BLAS implementations&c. This contrasts with project-specific custom pip build arguments. I don't even try to imagine how much extra complexity pip introduces in case of cross builds. Pip isn't designed to manage native builds, but meson is and seems to be great at the job.

I also see the documentation mention:

Meson will gain the capability to build wheels directly, but python -m build is going to become the standard way of doing this.

I find this suggestion confusing, because from my uninformed point of view this seems to be an anti-pattern.

Background: I've just spent some measly half an hour trying to figure out why NPY_LAPACK_ORDER (which is what I had thought scipy's setup.py looks at to guess if MKL should be used) is being ignored, and ended up just running meson directly. This works perfectly well, unless I use pip install --no-build-isolation later

@rgommers
Copy link
Member Author

rgommers commented May 3, 2023

@SomeoneSerge re NPY_LAPACK_ORDER and docs for how to do that with Meson, that is gh-18239. For the rest: wheels are expected for a Python package, we can't just not have them. Meson doesn't give them to you today, that's quite a ways off. So there is nothing we can do with your comments unfortunately - pip doesn't have the best UX for this kind of thing, but that is what it is.

@SomeoneSerge
Copy link

@rgommers oh, I didn't make myself quite clear. What worked in my case is the following sequence:

$ meson setup build/ ... -Dblas=mkl -Dlapack=mkl 
$ python -m pip wheel --config-settings builddir=build --verbose --no-index --no-build-isolation --no-clean --no-deps --wheel-dir dist .
$ python -m pip install ./*.whl ...

The --config-settings builddir=build escape hatch was already suggested here on github, but not in the documentation. To me this seems like a pretty decent solution: we let meson handle the native configuration, and we let pip generate the wheel with correct .dist-info. The flag explicitly tells pip that "the native part has already been taken care of, just run make and do python things". All tools do what they're best at.

How would you feel about including that in the documentation?

@rgommers
Copy link
Member Author

rgommers commented May 3, 2023

How would you feel about including that in the documentation?

It doesn't actually work reliably. The right way is documented at http://scipy.github.io/devdocs/dev/contributor/meson_advanced.html#select-a-different-blas-or-lapack-library. I'm rewriting the docs to make that info easier to find.

@SomeoneSerge
Copy link

Interesting. I read the page, but I'm still unsure what exactly can go wrong when using pip wheel --config-settings builddir=build, compared to -m build.

Could you elaborate on your reliability concerns? And regarding -m build, is there a way to make it respect the existing build/? Although I'm not asking because it's too hard to pass meson flags to -m build, but merely because this feels backwards

Thank you!

@rgommers
Copy link
Member Author

rgommers commented May 4, 2023

Could you elaborate on your reliability concerns?

Since meson-python 0.13.0, it will add --reconfigure to the meson setup invocation, so options may silently be dropped.

And regarding -m build, is there a way to make it respect the existing build/? Although I'm not asking because it's too hard to pass meson flags to -m build, but merely because this feels backwards

There is no way to do this, it is not supported. And there's no reason left I can think of to reuse a previously configured builds and the behavior would be ambiguous in some cases, so I think the "just don't do that" verdict is final.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meson Items related to the introduction of Meson as the new build system for SciPy
Projects
None yet
Development

No branches or pull requests

5 participants
@rgommers @Enchufa2 @eli-schwartz @SomeoneSerge and others