fortran-lang / stdlib

Fortran Standard Library
https://stdlib.fortran-lang.org
MIT License
1.02k stars 161 forks source link

Add findBLAS support to CMakeLists.txt #844

Open zoziha opened 3 days ago

zoziha commented 3 days ago

Description

(Related to stdlib/pull/772#issuecomment and MINGW-packages/pull/20874#issuecomment.)

The stdlib has recently seen significant developments in linear algebra support. In the downstream fortran-stdlib MSYS2 package, there are optimized BLAS links such as OpenBLAS. This PR is prepared to submit a CMakeLists.txt patch that supports findBLAS: If CMake finds other BLAS or LAPACK backends, it will preferentially link them; if CMake does not find other BLAS or LAPACK backends, the built-in BLAS and LAPACK versions of stdlib will be used.

Suggestions are welcome, thanks in advance. I'm not sure if CMake options like -DFIND_BLAS need to be added for this.

perazz commented 3 days ago

Thank you for this PR @zoziha. I have tested it on my laptop and it works well!

I believe that in the future, we may want to extend findBLAS and findLAPACK to check for 64-bit integer types to avoid potential issues with some versions of MKL. But for now it should be OK (CMake will search for a 32-bit version first if no options are specified).

I think it would be nice to add a couple of tests. I've tried to fork your repository to add them directly but it says "No available destinations to fork this repository.". So I've created a new branch on my repository and you can check it out here:

https://github.com/fortran-lang/stdlib/commit/68622098db3320a560ff9eb0414f9e8579a31a33#diff-33394812ba204689144fd2f80832db83853ba1cb32403edb4e15fe4893e675fd

@jalvesz has also worked on the build system and may have advice for us.

EDIT: I found how to add the tests into your PR branch, thanks.

jvdp1 commented 3 days ago

thank you @zoziha for this PR.

Note that C/CXX must be enabled for MKL (see here). Did you test it with MKL?

jalvesz commented 2 days ago

I think the tests and examples CMakeLists.txt should include if(BLAS_FOUND) if(LAPACK_FOUND)

There is also the BLA_VENDOR flag to activate as shown in the link shared by @jvdp1, there is some extra info in the latest cmake doc version : https://cmake.org/cmake/help/latest/module/FindBLAS.html (see at the end)

to check for 64-bit integer types to avoid potential issues with some versions of MKL

@perazz in the documentation they propose precisely:

set(BLA_VENDOR Intel10_64lp)

To make it point to a 64-bit version.

It could be a good idea to enable testing this within the CI:

for linux then having one of the gnu entries verify installation of blas/lapack

sudo apt-get install libblas-dev liblapack-dev

I'm not sure if CMake options like -DFIND_BLAS need to be added for this.

I think this is a good idea, the question is: what should the default be? I would be tempted to say that by default it should be TRUE.

Once this is running with CMake we can try to extend it with fpm as well.

perazz commented 2 days ago

@jalvesz couple thoughts:

Should become

#ifdef STDLIB_BLAS_ILP64
integer, parameter :: ilp = int64
#else
integer, parameter :: ilp = int32
#endif
jalvesz commented 2 days ago

@perazz totally agree, I just wanted to propose to have 1 or 2 jobs in the CI to test it, not to test all possible combinations as it would be too much and out of scope.

perazz commented 2 days ago

Absolutely! 💯