Skip to content

[cmake] use wfn91's linear algebra discovery modules #254

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

Merged
merged 17 commits into from
May 25, 2021

Conversation

evaleev
Copy link
Member

@evaleev evaleev commented Feb 11, 2021

No description provided.

@wavefunction91
Copy link

@evaleev This passes all CI except for Travis + clang (which is not a problem in the build, it's a problem in the Ubuntu packages). Do we want to fix the Travis build, or take Gitlab as being indicative of it working?

@evaleev
Copy link
Member Author

evaleev commented May 16, 2021

@wavefunction91 I think this is ready. I'm pretty sure my updates of the dox (INSTALL.md) are not 100% correct (does your kit respect BLA_STATIC?), but are mostly there

@evaleev evaleev requested a review from wavefunction91 May 16, 2021 14:44
@wavefunction91
Copy link

We have BLAS_PREFERS_STATIC, but I can add a BLA_STATIC compatibility if you'd prefer

- standard CMake BLAS/LAPACK modules.

The latter is used if CMake cache variable `BLA_VENDOR` is specified:
- `BLA_VENDOR` -- controls which vendor BLAS/LAPACK library will be sought

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use BLA_VENDOR, we set a priority list BLAS_PREFERENCE_LIST which will have the same effect as BLA_VENDOR when set to a single item. We do this to decouple LAPACK/BLAS (e.g. in the case of BLIS + NETLIB / BLIS + FLAME), but due to the hierarchy discovery, allowing full discovery (e.g. setting BLAS_PREFERENCE_LIST="IntelMKL" and letting FindLAPACK satisfy the FindBLAS dependency and recognize that it contains a LAPACK linker) will do the right thing.

We could add a BLA_VENDOR <-> BLAS_PREFERENCE_LIST compatibility, or you propagate in the VG CMake kit, which ever you prefer

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is least surprising to emulate Find{BLAS,LAPACK} as much as reasonable, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e. I think it's reasonable to read same input vars as those, i.e. read BLA_{VENDOR,STATIC}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that is the least surprising behaviour, I'll patch that today

Fortunately, Intel MKL can be discovered by BLAS++/LAPACK++ automatically in most instances; if needed, specifying `BLA_VENDOR` with [appropriate argument](https://cmake.org/cmake/help/latest/module/FindBLAS.html#input-variables) can be used to force TiledArray to use MKL. Unfortunately it is not possible to specify the use of TBB-based backend for MKL without the use of a toolchain file. All MKL-enabled toolchains in [The Valeev Group CMake kit](https://github.com/ValeevGroup/kit-cmake/tree/master/toolchains) can be used to configure TiledArray to use sequential, OpenMP, or TBB backend by setting the `MKL_THREADING` CMake cache variable to `SEQ`, `OMP`, or `TBB`, respectively. The toolchains also respect the user-provided choice of `BLA_STATIC`. If multiple MKL versions are present on your system, specify the apropriate variant of the library by loading the corresponding `mklvars.sh` script to set environment variables `MKLROOT` and, if necessary, `LD_LIBRARY_PATH`/`DYLD_LIBRARY_PATH`.
To discover and configure the use of Intel MKL consider these suggestions:
- The use of NWChemEx discovery kit is strongly recommended for discovering Intel MKL. The following CMake cache variables can be used to specify the desired Intel MKL configuration:
- `intelmkl_PREFERS_STATIC`: whether to look for static or shared/dynamic libraries (default = `OFF`)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be covered by BLAS_PREFERS_STATIC

@wavefunction91
Copy link

@evaleev Kitware compatibility is added in wavefunction91/linalg-cmake-modules@510b928 , we should just need to bump the version in vg-kit. This will allow for BLA_STATIC -> {BLAS,LAPACK,ScaLAPACK}_PREFERS_STATIC emulation. Same goes for BLA_VENDOR, but as we've discussed offline, this generally leads to ambiguity in the cases where BLAS/LAPACK interoperability is desired

The following are the allowed specs for XXX_PREFERENCE_LIST:

XXX=BLAS

  • ReferenceBLAS (NETLIB)
  • IntelMKL
  • IBMESSL
  • BLIS
  • OpenBLAS
  • Accelerate

XXX=LAPACK

  • ReferenceLAPACK (NETLIB)
  • FLAME

@evaleev
Copy link
Member Author

evaleev commented May 20, 2021

@wavefunction91 OK, thanks, will do

evaleev added a commit to ValeevGroup/kit-cmake that referenced this pull request May 25, 2021
@evaleev
Copy link
Member Author

evaleev commented May 25, 2021

@evaleev Kitware compatibility is added in wavefunction91/linalg-cmake-modules@510b928 , we should just need to bump the version in vg-kit.

done

The following are the allowed specs for XXX_PREFERENCE_LIST:

XXX=BLAS

  • ReferenceBLAS (NETLIB)
  • IntelMKL
  • IBMESSL
  • BLIS
  • OpenBLAS
  • Accelerate

XXX=LAPACK

  • ReferenceLAPACK (NETLIB)
  • FLAME

Does the user need to specify {LAPACK,ScaLAPACK}_PREFERENCE_LIST in the case BLAS_PREFERENCE_LIST= IntelMKL? I.e. does the latter imply that only IntelMKL will be considered when searching for LAPACK and ScaLAPACK?

@evaleev evaleev force-pushed the evaleev/feature/wfn91-linalg-discovery branch from 3989d45 to 3f8a4d1 Compare May 25, 2021 13:39
@wavefunction91
Copy link

@evaleev The user should not have to specify {Sca,}LAPACK_PREFERENCE_LIST for MKL. Take ScaLAPACK for example (since its the highest up on the stack) - if unguided discovery is used, then it will go like

  1. FindScaLAPACK -> no ScaLAPACK_LIBRARIES are set, so it looks for LAPACK
  2. FindLAPACK -> no LAPACK_LIBRARIES are set, looks for BLAS
  3. FindBLAS -> no BLAS_LIBRARIES are set, goes through BLAS_PREFERENCE_LIST, finds IntelMKL, propagates IntelMKL_LIBRARIES to BLAS_LIBRARIES. Because ScaLAPACK was requested, it also tacks on the BLACS/ScaLAPACK MKL linker (+ MPI), which is non-trivial (which is why it's important to ask for the highest level of the stack desired so it can propagate down to Find<VENDOR> if need be)
  4. FindLAPACK -> recognizes that it found BLAS, checks for LAPACK linker in BLAS_LIBRARIES, this will be successful for MKL and will propagate BLAS_LIBRARIES (and associated variables) to LAPACK_LIBRARIES
  5. FindScaLAPACK -> recognizes that it found LAPACK, checks for ScaLAPACK linker in LAPACK_LIBRARIES (which will work for MKL) and propagates to ScaLAPACK_LIBRARIES, etc.

@evaleev
Copy link
Member Author

evaleev commented May 25, 2021

@wavefunction91 sorry for wasting your time ... since the proper thing will be to interpret BLA_VENDOR rather than just passing it on as BLAS_PREFERENCE_LIST it's probably best to just use your kit's vars directly ... only realized this after seeing what happens when BLA_VENDOR used with standard CMake module is used with your kit: https://gitlab.com/ValeevGroup/tiledarray/-/jobs/1291849639#L538 ... I propose we avoid the use of BLA_VENDOR to avoid these kinds of issues.

@evaleev evaleev force-pushed the evaleev/feature/wfn91-linalg-discovery branch from e5b8bcc to 2e02078 Compare May 25, 2021 18:04
@evaleev evaleev merged commit e7f8f90 into master May 25, 2021
@evaleev evaleev deleted the evaleev/feature/wfn91-linalg-discovery branch May 25, 2021 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants