-
Notifications
You must be signed in to change notification settings - Fork 37
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
[Trivial] Add ability to use system packages #1171
Conversation
if (PARTHENON_USE_SYSTEM_PACKAGES) | ||
option(PARTHENON_IMPORT_KOKKOS | ||
"If ON, attempt to link to an external Kokkos library. If OFF, build Kokkos from source and package with Parthenon" | ||
ON) | ||
else() | ||
option(PARTHENON_IMPORT_KOKKOS | ||
"If ON, attempt to link to an external Kokkos library. If OFF, build Kokkos from source and package with Parthenon" | ||
OFF) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered using CMakeDependentOption
here, but I want IMPORT_KOKKOS
available and user-visible no matter which setting of PARTHENON_USE_SYSTEM_PACKAGES
was chosen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Maybe add a comment in building.rst
in the PARTHENON_IMPORT_KOKKOS
flag entry that it defaults to On
when PARTHENON_USE_SYSTEM_PACKAGES
is on.
PR Summary
In issues #1168 and #1169 @yurivict pointed out that the default parthenon configuration is designed for supercomputers where system packages are not used easily. This of course makes sense in our context. However it apparently raises issues for packaging on, e.g., a unix distro.
This MR resolves #1169 by adding the cmake option
PARTHENON_USE_SYSTEM_PACKAGES
which is set toOFF
by default. Enabling this automatically setsPARTHENON_IMPORT_KOKKOS
toON
and also removes all instances (of which there is currently one) ofNO_DEFAULT_PATH
infind_package
calls.PR Checklist