Skip to content

Commit a17970f

Browse files
committed
Compile MFEM externally
This commit puts MFEM on the same footing as the other dependencies. Instead of being unconditionally built, it is now build when building external dependencies. This allowed me to add MFEM as a dependency managed spack. In doing this, I found a new bug in spack: spack/spack#51505 So, I changed our approach to testing the spack package. Instead of adding a local repo, I overwrite palace in the `builtin` repo.
1 parent c7cd5ef commit a17970f

File tree

13 files changed

+1527
-54
lines changed

13 files changed

+1527
-54
lines changed

.github/workflows/docs.yml

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,19 @@ jobs:
3636
buildcache: true
3737
color: true
3838

39+
# https://github.com/spack/spack/issues/51505
40+
- name: Overwrite builtin Palace package
41+
run: |
42+
cp -r spack_repo/local/packages/palace "$(spack location --repo builtin)/packages"
43+
cp extern/patch/mfem/* "$(spack location --repo builtin)/packages/palace"
44+
3945
- name: Setup Environment
4046
run: |
4147
# Spack.yaml with most / all settings configured
4248
cat << EOF > spack.yaml
4349
spack:
4450
specs:
45-
- palace # we install this without a cache each time
51+
- palace@develop # we install this without a cache each time
4652
view: false
4753
config:
4854
install_tree:
@@ -56,8 +62,6 @@ jobs:
5662
packages:
5763
petsc:
5864
require: ~hdf5
59-
repos:
60-
- spack_repo/local
6165
mirrors:
6266
spack:
6367
binary: true
@@ -98,7 +102,7 @@ jobs:
98102
- name: Concretize Spack
99103
run: |
100104
# Using `spack develop` in order to have an in-source build
101-
spack -e . develop --path=$(pwd) local.palace@git."${{ github.head_ref || github.ref_name }}"=develop
105+
spack -e . develop --path=$(pwd) palace@git."${{ github.head_ref || github.ref_name }}"=develop
102106
spack -e . concretize -f
103107
104108
# Relies on cache-hit(s)
@@ -111,7 +115,7 @@ jobs:
111115
run: |
112116
spack -e . install --only-concrete --show-log-on-error --only package --keep-stage --no-cache
113117
# If you want to use the branch-local source instead (should we always do this?)
114-
# spack -e . develop --path=$(pwd) local.palace@git."${{ github.head_ref || github.ref_name }}"=develop
118+
# spack -e . develop --path=$(pwd) palace@git."${{ github.head_ref || github.ref_name }}"=develop
115119

116120
- name: Build and deploy
117121
env:

.github/workflows/spack.yml

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ jobs:
5151
compiler: [gcc] #, clang, intel]
5252
gpu: [none] #, cuda, rocm]
5353
blas: [openblas] # ,intel-oneapi-mkl]
54-
spack_version: [develop] # ,v0.23.1]
5554
mpi: [openmpi] #, mpich, mvapich]
55+
eigensolver: [arpack, slepc]
56+
solver: [mumps, strumpack, superlu-dist]
5657

5758
runs-on: palace_ubuntu-latest_16-core
5859
steps:
@@ -89,12 +90,7 @@ jobs:
8990
GPU_VARIANT=""
9091
fi
9192
92-
# Develop requires ^compiler, not %compiler
93-
if [[ "${{ matrix.spack_version }}" == 'v0.23.1' ]]; then
94-
PALACE_SPEC="local.palace@develop+strumpack+arpack+mumps+tests %${{ matrix.compiler }} ${GPU_VARIANT} ^${{ matrix.mpi }} ^${{ matrix.blas }}"
95-
else
96-
PALACE_SPEC="local.palace@develop+strumpack+arpack+mumps+tests${GPU_VARIANT} ^${{ matrix.mpi }} ^${{ matrix.blas }} ^${{ matrix.compiler }}"
97-
fi
93+
PALACE_SPEC="palace@develop+${{ matrix.solver }}+${{ matrix.eigensolver }} +tests %${{ matrix.compiler }} ${GPU_VARIANT} ^${{ matrix.mpi }} ^${{ matrix.blas }}"
9894
9995
# Spack.yaml with most / all settings configured
10096
cat << EOF > spack.yaml
@@ -116,8 +112,6 @@ jobs:
116112
require: ~hdf5
117113
rocblas:
118114
require: ~tensile
119-
repos:
120-
- spack_repo/local
121115
mirrors:
122116
spack:
123117
binary: true
@@ -131,6 +125,13 @@ jobs:
131125
secret_variable: GITHUB_TOKEN
132126
EOF
133127
128+
# https://github.com/spack/spack/issues/51505
129+
- name: Overwrite builtin Palace package
130+
if: needs.filter.outputs.test == 'true'
131+
run: |
132+
cp -r spack_repo/local/packages/palace "$(spack location --repo builtin)/packages"
133+
cp extern/patch/mfem/* "$(spack location --repo builtin)/packages/palace"
134+
134135
- name: Configure External Packages
135136
if: needs.filter.outputs.test == 'true'
136137
run: |
@@ -167,7 +168,7 @@ jobs:
167168
# Unfortunately it then becomes difficult to know when to re-concretize.
168169
run: |
169170
# Using `spack develop` in order to have an in-source build
170-
spack -e . develop --path=$(pwd) local.palace@git."${{ github.head_ref || github.ref_name }}"=develop
171+
spack -e . develop --path=$(pwd) palace@git."${{ github.head_ref || github.ref_name }}"=develop
171172
spack -e . concretize -f
172173
173174
- name: Build Dependencies
@@ -188,6 +189,14 @@ jobs:
188189
palace-unit-tests --skip-benchmarks
189190
mpirun -np 2 $(which palace-unit-tests) --skip-benchmarks
190191
192+
- uses: julia-actions/cache@v2
193+
if: needs.filter.outputs.test == 'true'
194+
195+
- uses: julia-actions/setup-julia@v2
196+
if: needs.filter.outputs.test == 'true'
197+
with:
198+
version: '1'
199+
191200
- name: Run Integration Tests
192201
if: needs.filter.outputs.test == 'true' && matrix.gpu == 'none' # Skip tests for GPU builds
193202
env:

CHANGELOG.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,23 @@ The format of this changelog is based on
6868
[PR 510](https://github.com/awslabs/palace/pull/510) and [PR 526](https://github.com/awslabs/palace/pull/526).
6969
- Update EM constants to CODATA Recommended Values of the Fundamental Physical Constants 2022
7070
[PR 525](https://github.com/awslabs/palace/pull/525).
71+
- Fix compatibility with CUDA 12 and GCC 14+. [PR 535](https://github.com/awslabs/palace/pull/535).
7172
- Scaled Rs/Ls/Cs of impedance boundary conditions affected by mesh cracking, fixing bug where
7273
`"CrackInternalBoundaryElements"` would lead to incorrect results. [PR 544](https://github.com/awslabs/palace/pull/544).
7374

75+
#### Build system
76+
77+
- When building on a machine with a GPU, `CMAKE_CUDA_ARCHITECTURES` has now a
78+
default value that is automatically determined to be the consistent with the
79+
GPU. [PR 530](https://github.com/awslabs/palace/pull/530).
80+
- The spack Palace build now supports a new variant `+tests` to compile and
81+
install the unit tests. [PR
82+
549](https://github.com/awslabs/palace/pull/549).
83+
- MFEM is no longer unconditionally compiled alongside of Palace and can now
84+
be built separately. This is used in the Palace spack build, which now
85+
explicitely depends on MFEM. [PR
86+
550](https://github.com/awslabs/palace/pull/550).
87+
7488
## [0.14.0] - 2025-08-20
7589

7690
- Added `--version` command line flag for displaying Palace version information.

CMakeLists.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,6 @@ if(PALACE_BUILD_EXTERNAL_DEPS)
212212
add_subdirectory("extern")
213213
endif()
214214

215-
# Add MFEM (always built as part of Palace)
216-
message(STATUS "====================== Configuring MFEM dependency =====================")
217-
include(ExternalMFEM)
218-
219215
# Add the main Palace project
220216
message(STATUS "========================== Configuring Palace ==========================")
221217
include(ExternalPalace)

cmake/ExternalPalace.cmake

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@
66
#
77

88
# Force build order
9-
set(PALACE_DEPENDENCIES mfem)
109
if(PALACE_BUILD_EXTERNAL_DEPS)
11-
list(APPEND PALACE_DEPENDENCIES libCEED json fmt eigen scn)
10+
list(APPEND PALACE_DEPENDENCIES mfem libCEED json fmt eigen scn)
1211
if(PALACE_WITH_SLEPC)
1312
list(APPEND PALACE_DEPENDENCIES slepc)
1413
endif()
@@ -24,12 +23,16 @@ list(APPEND PALACE_OPTIONS
2423
"-DPALACE_WITH_OPENMP=${PALACE_WITH_OPENMP}"
2524
"-DPALACE_WITH_SLEPC=${PALACE_WITH_SLEPC}"
2625
"-DPALACE_WITH_ARPACK=${PALACE_WITH_ARPACK}"
26+
"-DPALACE_WITH_SUNDIALS=${PALACE_WITH_SUNDIALS}"
27+
"-DPALACE_WITH_GSLIB=${PALACE_WITH_GSLIB}"
2728
"-DANALYZE_SOURCES_CLANG_TIDY=${ANALYZE_SOURCES_CLANG_TIDY}"
2829
"-DANALYZE_SOURCES_CPPCHECK=${ANALYZE_SOURCES_CPPCHECK}"
29-
"-DMFEM_DATA_PATH=${CMAKE_BINARY_DIR}/extern/mfem/data" # Path to meshes for testing
3030
"-DPALACE_BUILD_EXTERNAL_DEPS=${PALACE_BUILD_EXTERNAL_DEPS}" # For Catch2
3131
"-DPALACE_BUILD_WITH_COVERAGE=${PALACE_BUILD_WITH_COVERAGE}"
3232
)
33+
if(NOT "${MFEM_DIR}" STREQUAL "")
34+
list(APPEND PALACE_OPTIONS "-DMFEM_DIR=${MFEM_DIR}")
35+
endif()
3336
if(PALACE_WITH_ARPACK)
3437
list(APPEND PALACE_OPTIONS
3538
"-DCMAKE_Fortran_COMPILER=${CMAKE_Fortran_COMPILER}"

extern/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,7 @@ endif()
9393
message(STATUS "==================== Configuring libCEED dependency ====================")
9494
include(ExternalLibCEED)
9595

96+
# Add MFEM
97+
message(STATUS "====================== Configuring MFEM dependency =====================")
98+
include(ExternalMFEM)
99+

palace/CMakeLists.txt

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake")
112112
# Find MPI
113113
find_package(MPI REQUIRED)
114114

115+
116+
if(PALACE_WITH_SUNDIALS AND NOT PALACE_BUILD_EXTERNAL_DEPS)
117+
# Spack's sundials requires MPI with C bindings, so we alias
118+
# MPI_C to MPI_CXX (if it does not already exist).
119+
if(NOT TARGET MPI::MPI_C AND TARGET MPI::MPI_CXX)
120+
add_library(MPI::MPI_C ALIAS MPI::MPI_CXX)
121+
endif()
122+
endif()
123+
115124
# Find OpenMP
116125
if(PALACE_WITH_OPENMP)
117126
find_package(OpenMP REQUIRED)
@@ -141,15 +150,20 @@ if(NOT "${MFEM_DIR}" STREQUAL "")
141150
set(MFEM_ROOT ${MFEM_DIR})
142151
endif()
143152
find_package(Threads REQUIRED)
144-
find_package(MFEM REQUIRED CONFIG)
145-
message(STATUS "Found MFEM: ${MFEM_VERSION} in ${MFEM_DIR}")
146-
if(NOT MFEM_USE_MPI)
147-
message(FATAL_ERROR "Build requires MFEM with MPI support")
153+
154+
# MFEM in Spack is currently using autotools, and this find_package will fail
155+
# (hence the QUIET), but this is not a problem.
156+
find_package(MFEM QUIET CONFIG)
157+
if(MFEM_FOUND)
158+
message(STATUS "Found MFEM: ${MFEM_VERSION} in ${MFEM_DIR}")
159+
if(NOT MFEM_USE_MPI)
160+
message(FATAL_ERROR "Build requires MFEM with MPI support")
161+
endif()
162+
endif()
163+
164+
if(NOT "${HYPRE_DIR}" STREQUAL "")
165+
set(HYPRE_ROOT ${HYPRE_DIR})
148166
endif()
149-
# if(MFEM_CXX_FLAGS)
150-
# # Pull compiler flags from MFEM for OpenMP and optimizations
151-
# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${MFEM_CXX_FLAGS}")
152-
# endif()
153167

154168
# Find libCEED
155169
include(PkgConfigHelpers)
@@ -327,6 +341,29 @@ target_link_libraries(${LIB_TARGET_NAME}
327341
PUBLIC mfem ${LIBCEED_TARGET} nlohmann_json::nlohmann_json fmt::fmt scn::scn
328342
Eigen3::Eigen LAPACK::LAPACK MPI::MPI_CXX
329343
)
344+
345+
if(NOT PALACE_BUILD_EXTERNAL_DEPS)
346+
# When using external deps, we need to manually link them to Palace (when
347+
# building them, they get linked through mfem).
348+
349+
# TODO: Switch to find_package when moving to hypre3
350+
if(TARGET HYPRE::HYPRE)
351+
target_link_libraries(${LIB_TARGET_NAME} PUBLIC HYPRE::HYPRE)
352+
else()
353+
find_library(HYPRE_LIBRARY NAMES HYPRE)
354+
if(HYPRE_LIBRARY)
355+
target_link_libraries(${LIB_TARGET_NAME} PUBLIC ${HYPRE_LIBRARY})
356+
endif()
357+
endif()
358+
359+
# Explicitly link SUNDIALS if MFEM doesn't export it transitively
360+
if(PALACE_WITH_SUNDIALS)
361+
find_package(SUNDIALS REQUIRED)
362+
message(STATUS "Found SUNDIALS: ${SUNDIALS_VERSION} in ${SUNDIALS_DIR}")
363+
target_link_libraries(${LIB_TARGET_NAME} PUBLIC SUNDIALS::arkode SUNDIALS::cvode)
364+
endif()
365+
endif()
366+
330367
target_link_libraries(
331368
${LIB_TARGET_NAME}
332369
PRIVATE $<BUILD_INTERFACE:$<$<CONFIG:Debug,RelWithDebInfo>:common_warnings>>

spack_repo/local/packages/palace/package.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,33 @@ class Palace(CMakePackage, CudaPackage, ROCmPackage):
144144
depends_on("hypre~cuda", when="~cuda")
145145
depends_on("hypre~rocm", when="~rocm")
146146

147+
with when("@develop"):
148+
depends_on(
149+
"mfem+mpi+metis cxxstd=17 commit=0c4c006ef86dc2b2cf415e5bc4ed9118c9768652",
150+
patches=[
151+
"patch_mesh_vis_dev.diff",
152+
"patch_par_tet_mesh_fix_dev.diff",
153+
"patch_gmsh_parser_performance.diff",
154+
"patch_race_condition_fix.diff",
155+
],
156+
)
157+
depends_on("mfem+shared", when="+shared")
158+
depends_on("mfem~shared", when="~shared")
159+
depends_on("mfem+openmp", when="+openmp")
160+
depends_on("mfem~openmp", when="~openmp")
161+
depends_on("mfem+superlu-dist", when="+superlu-dist")
162+
depends_on("mfem~superlu-dist", when="~superlu-dist")
163+
depends_on("mfem+strumpack", when="+strumpack")
164+
depends_on("mfem~strumpack", when="~strumpack")
165+
depends_on("mfem+mumps", when="+mumps")
166+
depends_on("mfem~mumps", when="~mumps")
167+
depends_on("mfem+sundials", when="+sundials")
168+
depends_on("mfem~sundials", when="~sundials")
169+
depends_on("mfem+gslib", when="+gslib")
170+
depends_on("mfem~gslib", when="~gslib")
171+
172+
depends_on("mfem+exceptions", when="+tests")
173+
147174
with when("+libxsmm"):
148175
# NOTE: @=main != @main since libxsmm has a version main-2023-22
149176
depends_on("libxsmm@=main blas=0")
@@ -190,6 +217,7 @@ class Palace(CMakePackage, CudaPackage, ROCmPackage):
190217
for arch in CudaPackage.cuda_arch_values:
191218
cuda_variant = f"+cuda cuda_arch={arch}"
192219
depends_on(f"hypre{cuda_variant}", when=f"{cuda_variant}")
220+
depends_on(f"mfem{cuda_variant}", when=f"{cuda_variant}")
193221
depends_on(f"magma{cuda_variant}", when=f"{cuda_variant}")
194222
depends_on(f"libceed{cuda_variant}", when=f"{cuda_variant} @0.14:")
195223
depends_on(f"sundials{cuda_variant}", when=f"+sundials{cuda_variant} @0.14:")
@@ -200,6 +228,7 @@ class Palace(CMakePackage, CudaPackage, ROCmPackage):
200228
for arch in ROCmPackage.amdgpu_targets:
201229
rocm_variant = f"+rocm amdgpu_target={arch}"
202230
depends_on(f"hypre{rocm_variant}", when=f"{rocm_variant}")
231+
depends_on(f"mfem{rocm_variant}", when=f"{rocm_variant}")
203232
depends_on(f"magma{rocm_variant}", when=f"{rocm_variant}")
204233
depends_on(f"libceed{rocm_variant}", when=f"{rocm_variant} @0.14:")
205234
depends_on(f"sundials{rocm_variant}", when=f"+sundials{rocm_variant} @0.14:")
@@ -225,9 +254,12 @@ def cmake_args(self):
225254
self.define_from_variant("PALACE_WITH_SUNDIALS", "sundials"),
226255
self.define_from_variant("PALACE_WITH_SUPERLU", "superlu-dist"),
227256
self.define("PALACE_BUILD_EXTERNAL_DEPS", False),
228-
self.define_from_variant("PALACE_MFEM_USE_EXCEPTIONS", "tests"),
229257
]
230258

259+
260+
with when("@:0.14"):
261+
self.define_from_variant("PALACE_MFEM_USE_EXCEPTIONS", "tests")
262+
231263
# We guarantee that there are arch specs with conflicts above
232264
if self.spec.satisfies("+cuda"):
233265
args.append(

test/unit/CMakeLists.txt

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,13 @@ set_property(
8686

8787
# Add unit test mesh file path definition
8888
set_property(
89-
SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/test-libceed.cpp
89+
SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/test-geodata.cpp
90+
${CMAKE_CURRENT_SOURCE_DIR}/test-libceed.cpp
9091
${CMAKE_CURRENT_SOURCE_DIR}/test-materialoperator.cpp
9192
${CMAKE_CURRENT_SOURCE_DIR}/test-strattonchu.cpp
93+
${CMAKE_CURRENT_SOURCE_DIR}/test-rap.cpp
9294
APPEND PROPERTY COMPILE_DEFINITIONS "PALACE_TEST_MESH_DIR=\"${CMAKE_INSTALL_PREFIX}/share/palace/test/mesh\""
9395
)
94-
set_property(
95-
SOURCE
96-
${CMAKE_CURRENT_SOURCE_DIR}/test-geodata.cpp
97-
${CMAKE_CURRENT_SOURCE_DIR}/test-rap.cpp
98-
APPEND PROPERTY COMPILE_DEFINITIONS "MFEM_DATA_PATH=\"${CMAKE_INSTALL_PREFIX}/share/palace/test/mfem-data\""
99-
)
10096

10197
target_compile_definitions(
10298
unit-tests
@@ -128,14 +124,6 @@ install(
128124
DESTINATION share/palace/test
129125
)
130126

131-
# Install MFEM data files if available
132-
if(MFEM_DATA_PATH)
133-
install(
134-
DIRECTORY ${MFEM_DATA_PATH}/
135-
DESTINATION share/palace/test/mfem-data
136-
)
137-
endif()
138-
139127
install(
140128
DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/mesh/
141129
DESTINATION share/palace/test/mesh

0 commit comments

Comments
 (0)