Skip to content
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

ci: Spack docker containers #3039

Merged
merged 17 commits into from
Feb 12, 2025
Merged

ci: Spack docker containers #3039

merged 17 commits into from
Feb 12, 2025

Conversation

bmhan12
Copy link
Contributor

@bmhan12 bmhan12 commented Mar 14, 2024

This PR attempts to run Github Actions using Docker images with dependencies built with uberenv, spack.

Related to GEOS-DEV/thirdPartyLibs#261
Relates to GEOS-DEV/LvArray#318

@bmhan12 bmhan12 self-assigned this Mar 14, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from e88ad2b to ed09300 Compare March 28, 2024 21:02
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from 606df70 to eb9b77f Compare April 4, 2024 21:34
@bmhan12 bmhan12 added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Apr 18, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch 3 times, most recently from 9aa2678 to ada149c Compare April 18, 2024 18:42
@bmhan12 bmhan12 removed the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Apr 18, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch 3 times, most recently from d1a5ed4 to b9f7735 Compare April 18, 2024 21:58
@bmhan12 bmhan12 added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Apr 18, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from b9f7735 to f673591 Compare April 25, 2024 21:37
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from f673591 to a090c3b Compare May 9, 2024 16:10
@bmhan12 bmhan12 removed the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label May 16, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch 2 times, most recently from 3188c78 to ad53f07 Compare May 23, 2024 16:29
@bmhan12 bmhan12 added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label May 30, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch 2 times, most recently from 45f9942 to 76adca4 Compare May 30, 2024 18:21
@bmhan12 bmhan12 removed the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label May 30, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from 76adca4 to 52cdc7f Compare June 6, 2024 17:06
@bmhan12 bmhan12 added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Jun 6, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from 52cdc7f to ce09009 Compare June 6, 2024 18:37
@bmhan12 bmhan12 removed the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Jun 27, 2024
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from ce09009 to e8b3a32 Compare July 18, 2024 23:34
@bmhan12
Copy link
Contributor Author

bmhan12 commented Dec 13, 2024

Is it possible to replace the TPL_TAG stored in a hidden file inside this repo by a stable tag name inside the CI scripts ? Let's say "latest" ?

It is probably possible, but we would need to be careful in choosing which and when images get to be "latest".

@untereiner
Copy link
Contributor

Does it make sens to say that geos develop branch should be in sync with tpl develop branch (latest commit). But when doing a geos release a corresponding release has to be done on the tpl side ?

@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from a4a3e99 to be54b83 Compare January 17, 2025 00:23
Comment on lines +252 to +257
-DENABLE_HYPRE=${ENABLE_HYPRE} \
-DENABLE_HYPRE_DEVICE=${ENABLE_HYPRE_DEVICE} \
-DENABLE_TRILINOS=${ENABLE_TRILINOS} \
-DGEOS_LA_INTERFACE:PATH=${GEOS_LA_INTERFACE} \
Copy link
Collaborator

Choose a reason for hiding this comment

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

why the need to specify both ENABLE_TRILINOS and GEOS_LA_INTERFACE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why the need to specify both ENABLE_TRILINOS and GEOS_LA_INTERFACE?

The current way the linear algebra packages are configured in the CI jobs is by setting the ENABLE_HYPRE and ENABLE_TRILINOS variables as environment variables:

# The linear algebra environment variables (ENABLE_HYPRE, ENABLE_HYPRE_DEVICE & ENABLE_TRILINOS)
# could be passed as scripts parameters as well, but a specific care must be taken to be sure
# there's no conflict with the host-config files.
ENABLE_HYPRE=${{ inputs.ENABLE_HYPRE }}
ENABLE_HYPRE_DEVICE=${{ inputs.ENABLE_HYPRE_DEVICE }}
ENABLE_TRILINOS=${{ inputs.ENABLE_TRILINOS }}
docker_args+=(-e ENABLE_HYPRE=${ENABLE_HYPRE:-OFF})
docker_args+=(-e ENABLE_HYPRE_DEVICE=${ENABLE_HYPRE_DEVICE:-CPU})
docker_args+=(-e ENABLE_TRILINOS=${ENABLE_TRILINOS:-ON})

Then reading those environment variables into CMake and choosing a GEOS_LA_INTERFACE from there:

# If not defined as argument, take from the environment...
if(NOT DEFINED ENABLE_HYPRE)
set(ENABLE_HYPRE "$ENV{ENABLE_HYPRE}" CACHE BOOL "" FORCE)
endif()
# ... and then check the value.
if(ENABLE_HYPRE)
set(GEOS_LA_INTERFACE "Hypre" CACHE STRING "" FORCE)
else()
set(ENABLE_HYPRE OFF CACHE BOOL "" FORCE)
endif()
# Same pattern
if(NOT DEFINED ENABLE_TRILINOS)
set(ENABLE_TRILINOS "$ENV{ENABLE_TRILINOS}" CACHE BOOL "" FORCE)
endif()
if(ENABLE_TRILINOS)
set(GEOS_LA_INTERFACE "Trilinos" CACHE STRING "" FORCE)
else()
set(ENABLE_TRILINOS FALSE CACHE BOOL "" FORCE)
endif()
if( (ENABLE_HYPRE AND ENABLE_TRILINOS) OR (NOT ENABLE_TRILINOS AND NOT ENABLE_HYPRE))
MESSAGE(SEND_ERROR "Exactly one of ENABLE_HYPRE and ENABLE_TRILINOS must be defined.")
MESSAGE(SEND_ERROR "ENABLE_HYPRE = ${ENABLE_HYPRE}.")
MESSAGE(SEND_ERROR "ENABLE_TRILINOS = ${ENABLE_TRILINOS}.")
endif()
MESSAGE(STATUS "GEOS_LA_INTERFACE = ${GEOS_LA_INTERFACE}")

Instead of reading in environment variables from environment.cmake, the spack Docker containers each generate a host-config (spack-generated.cmake) to use instead, where the GEOS_LA_INTERFACE is by default set to Hypre. The snippet added here is for overriding the spack host-config defaults by defining those variables through CMake (passing them to config-build.py) instead of reading them through environment variables. This is done for CI jobs that have GEOS_LA_INTERFACE set to Trilinos.

@rrsettgast rrsettgast requested a review from Bubusch January 30, 2025 18:16
@bmhan12 bmhan12 force-pushed the feature/han12/docker_spack branch from be54b83 to 861477e Compare January 30, 2025 22:06
* `Manual compiler configuration <https://spack.readthedocs.io/en/latest/getting_started.html?highlight=compilers.yaml#manual-compiler-configuration>`_
* `External packages <https://spack.readthedocs.io/en/latest/packages_yaml.html#external-packages>`_

Building the dependencies can be as simple as running:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would remove this part about this generic command and start by saying that a spack-env file must be specified.

@rrsettgast rrsettgast merged commit 116560f into develop Feb 12, 2025
25 of 27 checks passed
@rrsettgast rrsettgast deleted the feature/han12/docker_spack branch February 12, 2025 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run code coverage enables running of the code coverage CI jobs ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: no rebaseline Does not require rebaseline flag: ready for review flag: requires updated submodule(s) flag: requires updated TPL(s) Needs a specific TPL PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants