Merge pull request #366 from awslabs/cameronrutherford/spack-cpu-build-fix

Spack CPU Build Fix. Add spack built libCEED support.

Author: hughcars

.github/workflows/spack.yml

@@ -10,68 +10,50 @@ concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 
+env:
+  # Note that each spack version needs it's own registry
+  SPACK_VERSION: develop
+  REGISTRY: ghcr.io/awslabs/palace
+  GITHUB_USER: ${{ github.actor }}
+  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
 jobs:
   build-and-test-spack:
     strategy:
-      fail-fast: false
+      fail-fast: false # don't have one build stop others from finishing
       matrix:
-        include:  # Include GPU build tests
-          - compiler: gcc
-            mpi: openmpi
-            gpu: none
-
-          # - compiler: gcc
-          #   mpi: mpich
-          #   gpu: cuda
-
-          # - compiler: gcc
-          #   mpi: mpich
-          #   gpu: rocm
+        compiler: [gcc] #, clang, intel]
+        gpu: [none] #, cuda, rocm]
+        blas: [openblas] # ,intel-oneapi-mkl]
+        spack_version: [develop] # ,v0.23.1]
+        mpi: [openmpi] #, mpich, mvapich]
 
     runs-on: palace_ubuntu-latest_16-core
     steps:
       - uses: actions/checkout@v4
 
-      - name: Configure Open MPI
-        if: matrix.mpi == 'openmpi'
-        run: |
-          sudo apt-get install -y openmpi-bin libopenmpi-dev
-
-      - name: Configure MPICH
-        if: matrix.mpi == 'mpich'
-        run: |
-          sudo apt-get install -y mpich libmpich-dev
-
-      - name: Configure Intel MPI
-        if: matrix.mpi == 'intelmpi'
-        uses: mpi4py/setup-mpi@v1
-        with:
-          mpi: ${{ matrix.mpi }}
-
       - name: Configure Clang compiler
         if: matrix.compiler == 'clang'
-        run: |
-          sudo apt-get install -y clang lld
+        run: sudo apt-get install -y clang lld
 
       - name: Configure Intel oneAPI compiler
         if: matrix.compiler == 'intel'
-        run: |
+        run:
           sudo apt-get install -y intel-oneapi-compiler-dpcpp-cpp=2024.2.1-1079 \
-                                  intel-oneapi-compiler-fortran=2024.2.1-1079
-
-      - uses: vsoch/spack-package-action/install@main
+          intel-oneapi-compiler-fortran=2024.2.1-1079
 
-      - name: Build Palace
+      - name: Setup Spack
+        uses: spack/setup-spack@v2
+        with:
+          ref: develop
+          buildcache: true
+          color: true
+          # This seems unecessary, but otherwise we clone into ./spack
+          # This then would override the spack local repo dir, and break CI
+          path: spack-path
+
+      - name: Setup Environment
         run: |
-          # Clean up Android SDK install (confuses Spack MKL link line?)
-          sudo rm -rf $ANDROID_HOME
-
-          # Set up Spack to use external packages (MPI, etc.) and local Palace package
-          # recipe
-          . /opt/spack/share/spack/setup-env.sh
-          spack external find --all
-          spack repo add spack/local
-
           # Configure GPU variant
           if [[ "${{ matrix.gpu }}" == 'cuda' ]]; then
             GPU_VARIANT="+cuda cuda_arch=70"
@@ -81,25 +63,99 @@ jobs:
             GPU_VARIANT=""
           fi
 
-          # Build and install
-          PALACE_SPEC="local.palace@develop%${{ matrix.compiler }}$GPU_VARIANT ^${{ matrix.mpi }}"
-          PALACE_SPEC="$PALACE_SPEC ^petsc~hdf5 ^intel-oneapi-mkl"
-          spack spec $PALACE_SPEC
-          spack dev-build $PALACE_SPEC
+          # Develop requires ^compiler, not %compiler
+          if [[ "${{ matrix.spack_version }}" == 'v0.23.1' ]]; then
+            PALACE_SPEC="local.palace@develop %${{ matrix.compiler }} ${GPU_VARIANT} ^${{ matrix.mpi }} ^${{ matrix.blas }}"
+          else
+            PALACE_SPEC="local.palace@develop${GPU_VARIANT} ^${{ matrix.mpi }} ^${{ matrix.blas }} ^${{ matrix.compiler }}"
+          fi
 
-      - name: Run tests
-        if: matrix.gpu == 'none'  # Skip tests for GPU builds
-        env:
-          NUM_PROC_TEST_MAX: '8'
+          # Spack.yaml with most / all settings configured
+          cat << EOF > spack.yaml
+          spack:
+            specs:
+              - ${PALACE_SPEC}
+            view: false
+            config:
+              install_tree:
+                root: /opt/spack
+                padded_length: False
+            concretizer:
+              reuse: false
+              unify: true
+              targets:
+                granularity: generic
+            packages:
+              petsc:
+                require: ~hdf5
+              rocblas:
+                require: ~tensile
+            repos:
+            - spack/local
+            mirrors:
+              spack:
+                binary: true
+                url: https://binaries.spack.io/${SPACK_VERSION}
+              local-buildcache:
+                binary: true
+                autopush: true
+                url: oci://${{ env.REGISTRY }}-${SPACK_VERSION}
+                signed: false
+                access_pair:
+                  id_variable: GITHUB_USER
+                  secret_variable: GITHUB_TOKEN
+          EOF
+
+      - name: Configure External Packages
         run: |
-          # Configure environment
-          export NUM_PROC_TEST=$(nproc 2> /dev/null || sysctl -n hw.ncpu)
-          if [[ "$NUM_PROC_TEST" -gt "$NUM_PROC_TEST_MAX" ]]; then
-            NUM_PROC_TEST=$NUM_PROC_TEST_MAX
-          fi
-          . /opt/spack/share/spack/setup-env.sh
-          spack load palace
+          # These cause build issues if built as externals
+          #   - python : often distributed python isn't feature complete / not all dependencies get detected
+          #   - OpenSSL / OpenSSH : since they are in /usr, spack struggles. It's common to rebuild these
+          #   - ncurses / bzip2 / xz / curl : caused build issues. We pull these from GHCR cache after first build
+          spack -e . external find --all \
+            --exclude openssl \
+            --exclude openssh \
+            --exclude python \
+            --exclude ncurses \
+            --exclude bzip2 \
+            --exclude xz \
+            --exclude curl
+
+      - name: Configure Compilers
+        run: spack -e . compiler find && spack -e . compiler list
+
+      - name: Configure Binary Mirror Keys
+        run: |
+          # If we cached these, that would be faster and safer
+          spack -e . buildcache keys --install --trust
 
+      - name: Bootstrap
+        run: spack -e . bootstrap now
+
+      - name: Concretize
+        # In theory we can re-use a concretization and pin a spack to speed this up.
+        # Unfortunately it then becomes difficult to know when to re-concretize.
+        run: |
+          # Using `spack develop` in order to have an in-source build
+          spack -e . develop --path=$(pwd) local.palace@git."${{ github.head_ref || github.ref_name }}"=develop
+          spack -e . concretize -f
+
+      - name: Build Dependencies
+        run: |
+          spack -e . install --only-concrete --no-check-signature --fail-fast --show-log-on-error --only dependencies
+
+      - name: Build Palace
+        # Build palace from source using this current directory
+        # We use `--no-cache` in order to force a rebuild
+        run: spack -e . install --only-concrete --keep-stage --show-log-on-error --only package --no-cache
+
+      # Should we run unit tests as well here?
+      - name: Run Integration Tests
+        if: matrix.gpu == 'none' # Skip tests for GPU builds
+        env:
+          NUM_PROC_TEST_MAX: "8"
+        run: |
+          eval $(spack -e . load --sh palace)
           # Run tests
           julia --project=test/examples -e 'using Pkg; Pkg.instantiate()'
           julia --project=test/examples --color=yes test/examples/runtests.jl

CHANGELOG.md

@@ -84,6 +84,8 @@ The format of this changelog is based on
     `MinimalRationalInterpolation` class to support multiple excitations. The MRI is unique to each
     excitation, but the PROM is shared between excitations.
   - Added `PortExcitations` to manage excitation pattern and print it to json metadata.
+  - Update spack configuration to address `0.12` API breakage between libxsmm and libCEED.
+  - Support spack vended libCEED and GSLIB builds.
 
 ## [0.13.0] - 2024-05-20
 

CMakeLists.txt

@@ -191,16 +191,6 @@ if(PALACE_BUILD_EXTERNAL_DEPS)
   add_subdirectory("extern")
 endif()
 
-# Add GSLIB (always built as part of Palace)
-if(PALACE_WITH_GSLIB)
-  message(STATUS "===================== Configuring GSLIB dependency =====================")
-  include(ExternalGSLIB)
-endif()
-
-# Add libCEED (always built as part of Palace)
-message(STATUS "==================== Configuring libCEED dependency ====================")
-include(ExternalLibCEED)
-
 # Add MFEM (always built as part of Palace)
 message(STATUS "====================== Configuring MFEM dependency =====================")
 include(ExternalMFEM)

cmake/ExternalBLASLAPACK.cmake

@@ -50,7 +50,6 @@ if(DEFINED ENV{ARMPL_DIR} OR DEFINED ENV{ARMPLROOT} OR DEFINED ENV{ARMPL_ROOT})
   set(BLA_VENDOR "Arm${ARMPL_LIB_SUFFIX}")
   find_package(BLAS REQUIRED)
   find_package(LAPACK REQUIRED)
-  set(LAPACK_LIBRARIES "${LAPACK_LIBRARIES};-lm")
 
   # Locate include directory
   find_path(_BLAS_LAPACK_INCLUDE_DIRS
@@ -186,10 +185,11 @@ else()
   find_path(_BLAS_LAPACK_INCLUDE_DIRS
     NAMES cblas.h
     HINTS ${_BLAS_LAPACK_DIRS}
-    PATH_SUFFIXES include include/openblas
+    PATH_SUFFIXES include include/openblas include/blis
     REQUIRED
   )
 endif()
+set(LAPACK_LIBRARIES "${LAPACK_LIBRARIES};-lm")
 
 # Save variables to cache
 set(_BLAS_LAPACK_LIBRARIES ${LAPACK_LIBRARIES} ${BLAS_LIBRARIES})

cmake/ExternalLibCEED.cmake

@@ -7,13 +7,11 @@
 
 # Force build order
 set(LIBCEED_DEPENDENCIES)
-if(PALACE_BUILD_EXTERNAL_DEPS)
-  if(PALACE_WITH_LIBXSMM)
-    list(APPEND LIBCEED_DEPENDENCIES libxsmm)
-  endif()
-  if(PALACE_WITH_MAGMA)
-    list(APPEND LIBCEED_DEPENDENCIES magma)
-  endif()
+if(PALACE_WITH_LIBXSMM)
+  list(APPEND LIBCEED_DEPENDENCIES libxsmm)
+endif()
+if(PALACE_WITH_MAGMA)
+  list(APPEND LIBCEED_DEPENDENCIES magma)
 endif()
 
 # Note on recommended flags for libCEED (from Makefile, Spack):
@@ -67,15 +65,9 @@ endif()
 
 # Configure libCEED backends (nvcc, hipcc flags are configured by libCEED)
 if(PALACE_WITH_LIBXSMM)
-  if(PALACE_BUILD_EXTERNAL_DEPS)
-    list(APPEND LIBCEED_OPTIONS
-      "XSMM_DIR=${CMAKE_INSTALL_PREFIX}"
-    )
-  else()
-    list(APPEND LIBCEED_OPTIONS
-      "XSMM_DIR=${LIBXSMM_DIR}"
-    )
-  endif()
+  list(APPEND LIBCEED_OPTIONS
+    "XSMM_DIR=${CMAKE_INSTALL_PREFIX}"
+  )
   # LIBXSMM can require linkage with BLAS for fallback
   if(NOT "${BLAS_LAPACK_LIBRARIES}" STREQUAL "")
     string(REPLACE "$<SEMICOLON>" " " LIBCEED_BLAS_LAPACK_LIBRARIES "${BLAS_LAPACK_LIBRARIES}")
@@ -107,15 +99,9 @@ if(PALACE_WITH_HIP)
   endif()
 endif()
 if(PALACE_WITH_MAGMA)
-  if(PALACE_BUILD_EXTERNAL_DEPS)
-    list(APPEND LIBCEED_OPTIONS
-      "MAGMA_DIR=${CMAKE_INSTALL_PREFIX}"
-    )
-  else()
-    list(APPEND LIBCEED_OPTIONS
-      "MAGMA_DIR=${MAGMA_DIR}"
-    )
-  endif()
+  list(APPEND LIBCEED_OPTIONS
+    "MAGMA_DIR=${CMAKE_INSTALL_PREFIX}"
+  )
 endif()
 
 string(REPLACE ";" "; " LIBCEED_OPTIONS_PRINT "${LIBCEED_OPTIONS}")

cmake/ExternalMFEM.cmake

@@ -20,12 +20,12 @@ if(PALACE_BUILD_EXTERNAL_DEPS)
   if(PALACE_WITH_SUNDIALS)
     list(APPEND MFEM_DEPENDENCIES sundials)
   endif()
+  if(PALACE_WITH_GSLIB)
+    list(APPEND MFEM_DEPENDENCIES gslib)
+  endif()
 else()
   set(MFEM_DEPENDENCIES)
 endif()
-if(PALACE_WITH_GSLIB)
-  list(APPEND MFEM_DEPENDENCIES gslib)
-endif()
 
 # Silence #pragma omp warnings when not building with OpenMP
 set(MFEM_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
@@ -151,10 +151,12 @@ endif()
 
 # MFEM with GSLIB is always built internally
 if(PALACE_WITH_GSLIB)
-  list(APPEND MFEM_OPTIONS
-    "-DMFEM_USE_GSLIB=YES"
-    "-DGSLIB_DIR=${CMAKE_INSTALL_PREFIX}"
-  )
+  list(APPEND MFEM_OPTIONS "-DMFEM_USE_GSLIB=YES")
+  if(PALACE_BUILD_EXTERNAL_DEPS)
+    list(APPEND MFEM_OPTIONS "-DGSLIB_DIR=${CMAKE_INSTALL_PREFIX}")
+  else()
+    list(APPEND MFEM_OPTIONS "-DGSLIB_DIR=${GSLIB_DIR}")
+  endif()
 endif()
 
 # Configure the rest of MFEM's dependencies

cmake/ExternalPalace.cmake

@@ -6,9 +6,9 @@
 #
 
 # Force build order
-set(PALACE_DEPENDENCIES mfem libCEED)
+set(PALACE_DEPENDENCIES mfem)
 if(PALACE_BUILD_EXTERNAL_DEPS)
-  list(APPEND PALACE_DEPENDENCIES json fmt eigen)
+  list(APPEND PALACE_DEPENDENCIES libCEED json fmt eigen)
   if(PALACE_WITH_SLEPC)
     list(APPEND PALACE_DEPENDENCIES slepc)
   endif()

extern/CMakeLists.txt

@@ -78,3 +78,14 @@ if(PALACE_WITH_SUNDIALS)
   message(STATUS "===================== Configuring SUNDIALS dependency =====================")
   include(ExternalSUNDIALS)
 endif()
+
+# Add GSLIB
+if(PALACE_WITH_GSLIB)
+  message(STATUS "===================== Configuring GSLIB dependency =====================")
+  include(ExternalGSLIB)
+endif()
+
+# Add libCEED
+message(STATUS "==================== Configuring libCEED dependency ====================")
+include(ExternalLibCEED)
+