Fix how haero is shipped as a CMake package - #495
Conversation
Also fix how haero looks for ekat
jeff-cohere
left a comment
jeff-cohere
left a comment
There was a problem hiding this comment.
Looks great! Thanks, @bartgol . Sorry about the previous structure--the role of Haero changed quite a bit over the course of the EAGLES project because we had so much trouble getting guidance on how to package mam4xx, so it became a bit of a mess after a while.
Should we test this with a mam4xx-enabled EAMxx build before we merge?
|
Yeah, no worries, it is a common theme with "fluid" projects, so some refactor is normal during the lifetime of a project (ekat had its good share of refactors too...). As for testing, I did test mam4xx with it, but you're right. I should at least build/test this infrastructure once. Let me update the haero/mam4xx submod in the ekat-update PR I have in eamxx, so we can see whether CI is happy. |
Kokkos takes care of that
|
@jeff-cohere I ran eamxx tests on my workstation with Cuda backend, and eamxx seems happy. I updated the e3sm PR with all submods up to date (including this branch for haero), so that we can test on all fronts. But if you are ok, once those tests pass, I can integrate this. Meanwhile, I'll also open a PR in mam4xx. |
mjschmidt271
left a comment
mjschmidt271
left a comment
There was a problem hiding this comment.
This looks great and appears to clean things up substantially
Thank you for doing this!
| message(STATUS "EKAT found on system--attempting to use local version.") | ||
| # set (CMAKE_FIND_DEBUG_MODE ON) | ||
| find_package(Ekat COMPONENTS Core KokkosUtils HINTS ${EKAT_PATH} | ||
| NO_CMAKE_PATH |
There was a problem hiding this comment.
I'm not familiar with these--they look like defensive measures to make sure haero doesn't look for ekat anywhere but where we tell it to look?
There was a problem hiding this comment.
Yes. I was getting an odd behavior on my computer, where find_pacakge would pick up an installation of ekat that was in the cmake registry (cmake can and often does keep a registry of installed pkgs for future use), so it wasn't picking up my mods as I updated ekat.
So yeah, the idea is "look for the pkg, but ONLY where I'm telling you, and ignore any other match around the system".
3 participants
This PR was needed b/c after the ekat update I was getting compilation errors in mam4xx. The reason was that haero was communicating the libraries to mam4xx via a list, and some were not necessarily imported targets. This PR makes haero more target-oriented, and ships it as a CMake package, so that mam4xx can just do
find_package(haero ...)and be done.I also cleaned up a bit the cmake organization, making less use of variables here and there, in favor of relying on target properties. E.g., the vars HAERO__FLAGS should be pointless, as flags are already propagated by targets, same for libraries and include directories. That is, provided that these properties are correctly set on the targets.
I know this PR is quite invasive, but I think it's worth it (and possibly could go further, but I think this is enough for our needs). The reason is that if we package haero correctly as a full-fledged cmake package, it is easy to integrate it downstream using "standard" cmake tpls search patterns (e.g.,
find_package), and without relying on peculiar cmake vars being set (which may not be obvious to those who did not setup the project). Put it another way, "standard" cmake packages are easy/quick to learn also for new users/maintainers of the project.