Convert blas/lapack/mkl to new cmake style #5972

jgfouca · 2023-10-05T22:08:16Z

This was a more challenging task than I thought it would. FindBLAS / FindLAPACK are built in to cmake and can also handle MKL stuff via the BLA_VENDOR setting (if you set it to Intel... you get various flavors of MKL). The find_package for blas/lapack is pretty smart and should do the right thing most of the time. Machine POCs should refine the settings on their machines using https://cmake.org/cmake/help/latest/module/FindBLAS.html#blas-lapack-vendors as as guide. I suspect many of these cache files were copy/pasted without a ton of consideration.

As an example, pm-cpu_nvidia is one platform where you will get DIFFs if you don't use the right blas/lapack. To set this up, I added the following to the environment block for that machine/compiler combo:

    <environment_variables compiler="nvidia">
      <env name="BLAS_ROOT">$SHELL{if [ -z "$BLAS_ROOT" ]; then echo /opt/nvidia/hpc_sdk/Linux_x86_64/22.7/compilers; else echo "$BLAS_ROOT"; fi}</env>
      <env name="LAPACK_ROOT">$SHELL{if [ -z "$LAPACK_ROOT" ]; then echo /opt/nvidia/hpc_sdk/Linux_x86_64/22.7/compilers; else echo "$LAPACK_ROOT"; fi}</env>
      <env name="BLA_VENDOR">NVHPC</env>
    </environment_variables>

The documentation indicates that the find modules for blas may be smart enough to auto detect the bla_vendor:

BLA_VENDOR
Set to one of the [BLAS/LAPACK Vendors] to search for BLAS only from the specified vendor.
If not set, all vendors are considered.

Once again, I am impressed at how much clutter is removed when we do things t "right" (using find_package instead of packing stuff into SLIBS) way.

[BFB]

bartgol · 2023-10-05T23:09:29Z

cime_config/machines/cmake_macros/gnu_melvin.cmake

@@ -11,4 +11,3 @@ endif()
 if (MPILIB STREQUAL mpi-serial AND NOT compile_threaded)
  set(PFUNIT_PATH "$ENV{SEMS_PFUNIT_ROOT}")
 endif()
-string(APPEND SLIBS " -lblas -llapack")


Is melvin still around? I thought it was no longer online.

Same thing for anlworkstation (already replaced by anlgce). Time to clean up these deprecated machines?

Yes, we badly need to go through all our configs and purge unused machines. I think we still have a lot of NCAR machine configs that we never use.

rljacob

Look at CircleCI fail.

jgfouca · 2023-10-06T16:53:37Z

@rljacob , I thought blas and lapack were required packages for E3SM? It looks like the circleci container doesn't install them? @lukaszlacinski

rljacob · 2023-10-06T16:57:58Z

That's a build-only test so it must have been possible to build without them.

rljacob

Circle CI fail is because container doesn't have blas/lapack. Need to redo container or switch to GH actions (with a newer container).

mahf708 · 2023-10-07T16:44:23Z

Circle CI fail is because container doesn't have blas/lapack.

The containers do ~~likely~~certainly have blas/lapack already. The cmake logic cannot find them for some reason :/

bartgol · 2023-10-08T03:15:46Z

@jgfouca perhaps we need to set the correct BLA_VENDOR variable in gnu_signularity.cmake?

We could also temporarily hack make the singularity build, to cat the e3sm.bldlog if case.build fails. That may show some more context from the cmake log.

mahf708 · 2023-10-08T03:53:24Z

Happy to help debug, but I am not really clear about the goals with the cmake changes yet...

FindBLAS / FindLAPACK are built in to cmake and can also handle MKL stuff via the BLA_VENDOR setting (if you set it to Intel... you get various flavors of MKL). For machines that looked like they were trying to use MKL in their cache files, I added set(BLA_VENDOR Intel10_64_dyn). I'm not 100% sure this is what we want in all cases, but it should at least get us building and running.

re this, could sophisticate our logic to handle all sorts of available ones and have precedence if desired (e.g., this)

mahf708 · 2023-10-08T13:22:07Z

One more comment: You could also link against "generic" BLAS/LAPACK and then switch the impl at runtime. That may be the solution you were hoping to find?

I'm not 100% sure this is what we want in all cases, but it should at least get us building and running.

mahf708

Some of your edits require cmake 3.18 (or higher). Could you at least set cmake_minimum_required(VERSION 3.18) where appropriate as part of this PR?

cime_config/machines/cmake_macros/gnu_bebop.cmake

components/cmake/build_model.cmake

github-actions · 2023-10-09T18:33:00Z

PR Preview Action v1.4.4
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-5972/
on branch `gh-pages` at 2023-10-11 21:36 UTC

mahf708 · 2023-10-09T18:38:39Z

For completeness, here are all the edits implemented in the new image. This takes me much closer to automating things, but I still need time.

mahf708/e3sm-imgs@de16688

notables:

add cmake to container
abstract stuff
remove sources

(this way, we also don't need a new machine in config_machines, we will just use the singularity one or we can rename it)

…lapack * origin/master: Fix nvidia_pm-cpu diffs for some tests Fix PIO dep on mpi-serial add ghp preview

Convert blas/lapack/mkl to new cmake style This was a more challenging task than I thought it would. FindBLAS / FindLAPACK are built in to cmake and can also handle MKL stuff via the BLA_VENDOR setting (if you set it to Intel... you get various flavors of MKL). For machines that looked like they were trying to use MKL in their cache files, I added set(BLA_VENDOR Intel10_64_dyn). I'm not 100% sure this is what we want in all cases, but it should at least get us building and running. Machine POCs should refine the settings on their machines using https://cmake.org/cmake/help/latest/module/FindBLAS.html#blas-lapack-vendors as as guide. I suspect many of these cache files were copy/pasted without a ton of consideration. The documentation indicates that the find modules for blas may be smart enough to auto detect the bla_vendor: BLA_VENDOR Set to one of the [BLAS/LAPACK Vendors] to search for BLAS only from the specified vendor. If not set, all vendors are considered. Once again, I am impressed at how much clutter is removed when we do things t "right" (using find_package instead of packing stuff into SLIBS) way. [BFB]

Get rid of all the BLA_VENDOR stuff in the macros. It had no impact because deps are loading before macros are processed. We should prefer using the environment to set this stuff like we are doing for the other libs. pm-cpu_nvidia seems to be the one platform sensitive to lapack/blas so can be used as an example of how to specify these libraries.

Merge 2 for this PR. * jgfouca/cmake_blas_lapack: blas/lapack settings should not depend on mpilib Refactor blas and lapack stuff a bit more

Merge 3 for this PR.

Convert blas/lapack/mkl to new cmake style This was a more challenging task than I thought it would. FindBLAS / FindLAPACK are built in to cmake and can also handle MKL stuff via the BLA_VENDOR setting (if you set it to Intel... you get various flavors of MKL). The find_package for blas/lapack is pretty smart and should do the right thing most of the time. Machine POCs should refine the settings on their machines using https://cmake.org/cmake/help/latest/module/FindBLAS.html#blas-lapack-vendors as as guide. I suspect many of these cache files were copy/pasted without a ton of consideration. As an example, pm-cpu_nvidia is one platform where you will get DIFFs if you don't use the right blas/lapack. To set this up, I added the following to the environment block for that machine/compiler combo: <environment_variables compiler="nvidia"> <env name="BLAS_ROOT">$SHELL{if [ -z "$BLAS_ROOT" ]; then echo /opt/nvidia/hpc_sdk/Linux_x86_64/22.7/compilers; else echo "$BLAS_ROOT"; fi}</env> <env name="LAPACK_ROOT">$SHELL{if [ -z "$LAPACK_ROOT" ]; then echo /opt/nvidia/hpc_sdk/Linux_x86_64/22.7/compilers; else echo "$LAPACK_ROOT"; fi}</env> <env name="BLA_VENDOR">NVHPC</env> </environment_variables> The documentation indicates that the find modules for blas may be smart enough to auto detect the bla_vendor: BLA_VENDOR Set to one of the [BLAS/LAPACK Vendors] to search for BLAS only from the specified vendor. If not set, all vendors are considered. Once again, I am impressed at how much clutter is removed when we do things t "right" (using find_package instead of packing stuff into SLIBS) way. [BFB]

Convert blas/lapack/mkl to new cmake style

9f6e496

jgfouca self-assigned this Oct 5, 2023

jgfouca requested review from bartgol and dqwu October 5, 2023 22:08

bartgol approved these changes Oct 5, 2023

View reviewed changes

rljacob requested changes Oct 6, 2023

View reviewed changes

rljacob approved these changes Oct 6, 2023

View reviewed changes

jgfouca mentioned this pull request Oct 5, 2023

E3SM CMake Build System V2! #5943

Open

49 tasks

mahf708 force-pushed the jgfouca/cmake_blas_lapack branch from 8f04835 to 6c0a316 Compare October 7, 2023 16:27

This comment was marked as outdated.

Sign in to view

mahf708 force-pushed the jgfouca/cmake_blas_lapack branch from 6c0a316 to 9f6e496 Compare October 7, 2023 16:41

mahf708 requested changes Oct 8, 2023

View reviewed changes

mahf708 reviewed Oct 8, 2023

View reviewed changes

cime_config/machines/cmake_macros/gnu_bebop.cmake Outdated Show resolved Hide resolved

mahf708 reviewed Oct 8, 2023

View reviewed changes

components/cmake/build_model.cmake Show resolved Hide resolved

use updated container with singularity

d936aee

mahf708 force-pushed the jgfouca/cmake_blas_lapack branch from b94f5bc to d936aee Compare October 8, 2023 23:17

rljacob added the CMake build system label Oct 9, 2023

Update minimum cmake

4d82d99

mahf708 approved these changes Oct 9, 2023

View reviewed changes

dqwu approved these changes Oct 9, 2023

View reviewed changes

Merge remote-tracking branch 'origin/master' into jgfouca/cmake_blas_…

9b40b2d

…lapack * origin/master: Fix nvidia_pm-cpu diffs for some tests Fix PIO dep on mpi-serial add ghp preview

jgfouca added 2 commits October 11, 2023 15:30

blas/lapack settings should not depend on mpilib

26170af

jgfouca added a commit that referenced this pull request Oct 11, 2023

Merge branch 'jgfouca/cmake_blas_lapack' into next (PR #5972)

7b5906d

Merge 2 for this PR. * jgfouca/cmake_blas_lapack: blas/lapack settings should not depend on mpilib Refactor blas and lapack stuff a bit more

bartgol approved these changes Oct 12, 2023

View reviewed changes

jgfouca added a commit that referenced this pull request Oct 12, 2023

Merge branch 'jgfouca/cmake_blas_lapack' into next (PR #5972)

6e792a8

Merge 3 for this PR.

jgfouca merged commit 26170af into master Oct 12, 2023
2 checks passed

jgfouca deleted the jgfouca/cmake_blas_lapack branch October 12, 2023 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert blas/lapack/mkl to new cmake style #5972

Convert blas/lapack/mkl to new cmake style #5972

jgfouca commented Oct 5, 2023 •

edited

Loading

bartgol Oct 5, 2023

dqwu Oct 6, 2023

jgfouca Oct 6, 2023

rljacob left a comment

jgfouca commented Oct 6, 2023

rljacob commented Oct 6, 2023

rljacob left a comment

This comment was marked as outdated.

mahf708 commented Oct 7, 2023 •

edited

Loading

bartgol commented Oct 8, 2023

mahf708 commented Oct 8, 2023 •

edited

Loading

mahf708 commented Oct 8, 2023

mahf708 left a comment

github-actions bot commented Oct 9, 2023 •

edited

Loading

mahf708 commented Oct 9, 2023

Convert blas/lapack/mkl to new cmake style #5972

Convert blas/lapack/mkl to new cmake style #5972

Conversation

jgfouca commented Oct 5, 2023 • edited Loading

bartgol Oct 5, 2023

Choose a reason for hiding this comment

dqwu Oct 6, 2023

Choose a reason for hiding this comment

jgfouca Oct 6, 2023

Choose a reason for hiding this comment

rljacob left a comment

Choose a reason for hiding this comment

jgfouca commented Oct 6, 2023

rljacob commented Oct 6, 2023

rljacob left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

mahf708 commented Oct 7, 2023 • edited Loading

bartgol commented Oct 8, 2023

mahf708 commented Oct 8, 2023 • edited Loading

mahf708 commented Oct 8, 2023

mahf708 left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 9, 2023 • edited Loading

mahf708 commented Oct 9, 2023

jgfouca commented Oct 5, 2023 •

edited

Loading

mahf708 commented Oct 7, 2023 •

edited

Loading

mahf708 commented Oct 8, 2023 •

edited

Loading

github-actions bot commented Oct 9, 2023 •

edited

Loading