CMake: Improve handling of Albany and related packages #5950

jgfouca · 2023-09-22T21:16:47Z

Specifically:

Albany
Trilinos
Kokkos

This PR establishes the new paradigm of setting ${Package}_ROOT to find the packages we need. So far, I am very happy with how this is going. You can see how much cleaner it is to do things the CMake-3 so you don't have to micro-manage flags.

This PR is currently WIP because I was not able to fully test it on pm-cpu due to permissions issues with the boost that was used to build Trilinos.

In order to be consistent, changes kokkos override env var from KOKKOS_PATH to Kokkos_ROOT.

Fixes #5786

jgfouca · 2023-09-22T21:18:07Z

cime_config/machines/config_machines.xml

@@ -257,6 +257,8 @@
      <env name="PERL5LIB">/global/cfs/cdirs/e3sm/perl/lib/perl5-only-switch</env>
      <env name="FI_CXI_RX_MATCH_MODE">software</env>
      <env name="MPICH_COLL_SYNC">MPI_Bcast</env>
+      <env name="Albany_ROOT">$SHELL{if [ -z "$Albany_ROOT" ]; then echo /global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc; else echo "$Albany_ROOT"; fi}</env>


The pattern for setting up ${Package}_ROOT is established here. It's a bit messy so I'm wondering if it would be better to move this login into the cmake macro file for the machine?

There's a better bash syntax:

Albany_ROOT=${Albany_ROOT:=/global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc}

The ${VAR:=default} syntax expands to ${VAR} if VAR is defined, otherwise it expands to default. E.g.:

$ export FOO=foo $ export BAR=${FOO:=bar} && echo $BAR foo $ export BAR={FOOBAR:=bar} && echo $BAR bar

@bartgol , I tried that at first. The problem is that the CIME regex will incorrectly match the first } as terminating the SHELL command, so you can use that character in your syntax.

bartgol

Looks much cleaner. I left a suggestion for the bash env var. I think it may be a bit cleaner, but up to you.

bartgol · 2023-09-25T15:09:28Z

cime_config/machines/config_machines.xml

@@ -257,6 +257,8 @@
      <env name="PERL5LIB">/global/cfs/cdirs/e3sm/perl/lib/perl5-only-switch</env>
      <env name="FI_CXI_RX_MATCH_MODE">software</env>
      <env name="MPICH_COLL_SYNC">MPI_Bcast</env>
+      <env name="Albany_ROOT">$SHELL{if [ -z "$Albany_ROOT" ]; then echo /global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc; else echo "$Albany_ROOT"; fi}</env>


There's a better bash syntax:

Albany_ROOT=${Albany_ROOT:=/global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc}

The ${VAR:=default} syntax expands to ${VAR} if VAR is defined, otherwise it expands to default. E.g.:

$ export FOO=foo $ export BAR=${FOO:=bar} && echo $BAR foo $ export BAR={FOOBAR:=bar} && echo $BAR bar

bartgol · 2023-09-25T15:09:54Z

components/cmake/common_setup.cmake

@@ -205,65 +205,6 @@ if (USE_PETSC)
  set(PETSC_LIB ${PETSC_LIBRARIES})
 endif()

-if (USE_TRILINOS)


So much better than what we had...

bartgol · 2023-09-25T15:11:52Z

components/cmake/find_dep_packages.cmake

+endif()
+
+# Albany depends on Trilinos
+if (USE_ALBANY OR USE_TRILINOS)


This is fine, but for the record, I think that USE_ALBANY here is redundant. find_package(ALBANY REQUIRED) will also call find_package(Trilinos REQUIRED). If a CMake package is installed well, it is responsible of finding all its TPLs when you call find_package(YOUR_PKG).

It looks like this is a deficiency with the Albany cmake system. I tried without USE_ALBANY check for Trilinos and I got this error:

CMake Error at /global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc/lib64/Albany/cmake/albany-targets.cmake:61 (set_target_properties): The link interface of target "albanyLib" contains: Panzer::all_libs but the target was not found. Possible reasons include: * There is a typo in the target name. * A find_package call is missing for an IMPORTED target. * An ALIAS target is missing. Call Stack (most recent call first): /global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc/lib64/Albany/cmake/AlbanyConfig.cmake:28 (include) cmake/find_dep_packages.cmake:45 (find_package) CMakeLists.txt:124 (include)

Ah, we are probably not packaging albany that well.. :/

I am going through our cmake stuff, and indeed we lack a lot of stuff to consider us "good cmake citizens"... I will fix it at some point.

jgfouca · 2023-09-25T20:03:32Z

@mperego , there are some deficiencies in the Albany Cmake system that are causing me problems:

Albany's find_package does not find Trilinos, so E3SM has to do it. It's easy enough for E3SM to do it, so this is not a show stopper.
For some reason, the mpasInterface library is linked via -lmpasInterface instead of just listing the full path to the library like the other Albany libraries do. This causes a failure cannot find -lmpasInterface: No such file or directory since the albany cmake doesn't add that directory via -L

If I fix (2) by manually editing the link line (replace -lmpasInterface with /global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc/lib64/libmpasInterface.so, then everything works and it builds.

Since these problems aren't related to E3SM, I'm going to push ahead with this PR unless you say otherwise.

jgfouca · 2023-09-25T20:14:06Z

I removed all instances of ALBANY_PATH from the Cmake macros. This means that the BGWCYCL1850 compset will only work on perlmutter currently. Any machine that wants to support this compset will need to follow the pattern in the perlmutter config_machines and add Albany_ROOT and Trilinos_ROOT.

bartgol · 2023-09-25T20:26:37Z

@mperego , there are some deficiencies in the Albany Cmake system that are causing me problems:

Albany's find_package does not find Trilinos, so E3SM has to do it. It's easy enough for E3SM to do it, so this is not a show stopper.

For some reason, the mpasInterface library is linked via -lmpasInterface instead of just listing the full path to the library like the other Albany libraries do. This causes a failure cannot find -lmpasInterface: No such file or directory since the albany cmake doesn't add that directory via -L

If I fix (2) by manually editing the link line (replace -lmpasInterface with /global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc/lib64/libmpasInterface.so, then everything works and it builds.

Since these problems aren't related to E3SM, I'm going to push ahead with this PR unless you say otherwise.

We're fixing other issues with our cmake logic (lots of legacy stuff, and some not-so-great cmake usage here and there). I'll keep in mind these issues in the process.

CMake: Improve handling of Albany and related packages Specifically: * Albany * Trilinos * Kokkos This PR establishes the new paradigm of setting ${Package}_ROOT to find the packages we need. So far, I am very happy with how this is going. You can see how much cleaner it is to do things the CMake-3 so you don't have to micro-manage flags. In order to be consistent, changes kokkos override env var from KOKKOS_PATH to Kokkos_ROOT. Fixes #5786 [BFB] * jgfouca/cmake_albany: Remove obsolete references to ALBANY_PATH in cmake macros Improve Kokkos handling CMake: Improve handling of Albany and related packages

jgfouca · 2023-09-25T20:29:27Z

Thanks, @bartgol ! Merged to next.

mperego · 2023-09-25T20:35:21Z

Thanks a lot @jgfouca! This looks so much cleaner. Looking forward to trying this out.

bartgol · 2023-11-03T16:22:35Z

@mperego , there are some deficiencies in the Albany Cmake system that are causing me problems:

Albany's find_package does not find Trilinos, so E3SM has to do it. It's easy enough for E3SM to do it, so this is not a show stopper.

For some reason, the mpasInterface library is linked via -lmpasInterface instead of just listing the full path to the library like the other Albany libraries do. This causes a failure cannot find -lmpasInterface: No such file or directory since the albany cmake doesn't add that directory via -L

If I fix (2) by manually editing the link line (replace -lmpasInterface with /global/common/software/e3sm/mali_tpls/albany-e3sm-serial-release-gcc/lib64/libmpasInterface.so, then everything works and it builds.

Since these problems aren't related to E3SM, I'm going to push ahead with this PR unless you say otherwise.

I found the culprit. Albany was not exporting the mpasInterface target (unlike the other libs), so it was not available as a CMake target to downstream projects. Will fix it in Albany.

The lack of export was causing link errors in E3SM, as noted in E3SM-Project/E3SM#5950.

jgfouca added 2 commits September 22, 2023 15:03

CMake: Improve handling of Albany and related packages

ecb3576

Improve Kokkos handling

c4ecf63

jgfouca added BFB PR leaves answers BFB CMake build system labels Sep 22, 2023

jgfouca requested review from bartgol and mperego September 22, 2023 21:16

jgfouca self-assigned this Sep 22, 2023

jgfouca commented Sep 22, 2023

View reviewed changes

jgfouca mentioned this pull request Sep 22, 2023

E3SM CMake Build System V2! #5943

Open

49 tasks

jgfouca requested review from rljacob and jasonb5 September 22, 2023 21:19

bartgol approved these changes Sep 25, 2023

View reviewed changes

Remove obsolete references to ALBANY_PATH in cmake macros

c5ad9f9

jgfouca merged commit ffc0e2b into master Sep 26, 2023
2 checks passed

jgfouca deleted the jgfouca/cmake_albany branch September 26, 2023 16:22

bartgol added a commit to sandialabs/Albany that referenced this pull request Nov 3, 2023

Export mpasInterface target when installing

ba26e67

The lack of export was causing link errors in E3SM, as noted in E3SM-Project/E3SM#5950.

mperego pushed a commit to sandialabs/Albany that referenced this pull request Nov 3, 2023

Export mpasInterface target when installing

5082677

The lack of export was causing link errors in E3SM, as noted in E3SM-Project/E3SM#5950.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMake: Improve handling of Albany and related packages #5950

CMake: Improve handling of Albany and related packages #5950

jgfouca commented Sep 22, 2023 •

edited

Loading

jgfouca Sep 22, 2023

bartgol Sep 25, 2023

jgfouca Sep 25, 2023

bartgol left a comment

bartgol Sep 25, 2023

bartgol Sep 25, 2023

bartgol Sep 25, 2023

jgfouca Sep 25, 2023

bartgol Sep 25, 2023

bartgol Sep 25, 2023

jgfouca commented Sep 25, 2023

jgfouca commented Sep 25, 2023

bartgol commented Sep 25, 2023

jgfouca commented Sep 25, 2023

mperego commented Sep 25, 2023

bartgol commented Nov 3, 2023

CMake: Improve handling of Albany and related packages #5950

CMake: Improve handling of Albany and related packages #5950

Conversation

jgfouca commented Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartgol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgfouca commented Sep 25, 2023

jgfouca commented Sep 25, 2023

bartgol commented Sep 25, 2023

jgfouca commented Sep 25, 2023

mperego commented Sep 25, 2023

bartgol commented Nov 3, 2023

jgfouca commented Sep 22, 2023 •

edited

Loading