Merge pull request #6425 from nilsvu/kokkos

Improve NVCC & Kokkos support, test on CI

.github/actions/parse-compiler/action.yaml

@@ -16,16 +16,21 @@ runs:
   using: "composite"
   steps:
     - run: |
-        if [[ ${{ inputs.compiler }} =~ (gcc|clang)-([0-9\.]+) ]]; then
+        if [[ ${{ inputs.compiler }} =~ (gcc|clang|nvcc)-([0-9\.\-]+) ]]; then
           COMPILER_ID=${BASH_REMATCH[1]}
           COMPILER_VERSION=${BASH_REMATCH[2]}
-          CC=${COMPILER_ID}-${COMPILER_VERSION};
           if [[ $COMPILER_ID = gcc ]]; then
+            CC=gcc-${COMPILER_VERSION};
             CXX=g++-${COMPILER_VERSION};
             FC=gfortran-${COMPILER_VERSION};
-          else
+          elif [[ $COMPILER_ID = clang ]]; then
+            CC=clang-${COMPILER_VERSION};
             CXX=clang++-${COMPILER_VERSION};
             FC=gfortran-11;
+          elif [[ $COMPILER_ID = nvcc ]]; then
+            CC=gcc;
+            CXX=nvcc;
+            FC=gfortran-11;
           fi
         fi
         echo "CC=$CC" >> $GITHUB_ENV

.github/workflows/Tests.yaml

@@ -492,16 +492,26 @@ jobs:
             # Test `install` target with clang in Release mode because it uses
             # little disk space
             install: ON
+          - compiler: nvcc-12-6
+            CUDA: ON
+            # Compile in Release mode to speed up the build
+            build_type: Release
+            # Build only a subset of the code because compiling the full code is
+            # too slow and exceeds the available memory. We should extend this
+            # to the full code once we have a better solution for the memory
+            # issue. See 'Test nvcc support' step below.
+            unit_tests: OFF
+            test_executables: OFF
+            # Enable PlaneWave3D.yaml input file test
+            input_file_tests_min_priority: "normal"
+            # Disable pybindings until they support nvcc
+            BUILD_PYTHON_BINDINGS: OFF
+            # Disable warnings as nvcc emits a lot of them from system headers
+            ENABLE_WARNINGS: OFF
 
     container:
       image: ${{ inputs.container || 'sxscollaboration/spectre:dev' }}
       env:
-        # We run unit tests with the following compiler flags:
-        # - `-Werror`: Treat warnings as error.
-        # - `-march=x86-64`: Make sure we are building on a consistent
-        #   architecture so caching works. This is necessary because GitHub
-        #   may run the job on different hardware.
-        CXXFLAGS: "-Werror"
         # We make sure to use a fixed absolute path for the ccache directory
         CCACHE_DIR: /work/ccache
         # Use a separate temp directory to conserve cache space on GitHub
@@ -551,8 +561,19 @@ jobs:
           apt-get update -y
           if [[ $COMPILER_ID = gcc ]]; then
             apt-get install -y $CC $CXX $FC
-          else
+          elif [[ $COMPILER_ID = clang ]]; then
             apt-get install -y $CC $FC
+          elif [[ $COMPILER_ID = nvcc ]]; then
+            # Install CUDA
+            wget https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2204/x86_64/cuda-keyring_1.1-1_all.deb
+            dpkg -i cuda-keyring_1.1-1_all.deb
+            apt-get update -y
+            apt-get install -y cuda-nvcc-${COMPILER_VERSION}
+            echo "/usr/local/cuda-${COMPILER_VERSION/-/.}/bin" >> $GITHUB_PATH
+            # Install Kokkos compiler wrapper
+            git clone https://github.com/kokkos/kokkos
+            CXX=$PWD/kokkos/bin/nvcc_wrapper
+            echo "CXX=$CXX" >> $GITHUB_ENV
           fi
       # Install specific CMake version if requested
       - name: Install CMake version
@@ -632,9 +653,17 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
         # Notes on the build configuration:
         # - We don't need debug symbols during CI, so we turn them off to reduce
         #   memory usage.
+        # - We run unit tests with the following compiler flags:
+        #   -Werror: Treat warnings as error.
+        #   -march=x86-64: Make sure we are building on a consistent
+        #   architecture so caching works. This is necessary because GitHub may
+        #   run the job on different hardware.
         run: >
           mkdir build && cd build
 
+          ENABLE_WARNINGS=${{ matrix.ENABLE_WARNINGS }}
+          WERROR="${{ matrix.WERROR != 'OFF' && '-Werror' || '' }}"
+          CXX_FLAGS="${WERROR} ${{ matrix.EXTRA_CXX_FLAGS }}"
           BUILD_PYTHON_BINDINGS=${{ matrix.BUILD_PYTHON_BINDINGS }}
           BUILD_SHARED_LIBS=${{ matrix.BUILD_SHARED_LIBS }}
           MATRIX_CHARM_ROOT=${{ matrix.CHARM_ROOT }}
@@ -654,8 +683,9 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
           -D CMAKE_C_COMPILER=${CC}
           -D CMAKE_CXX_COMPILER=${CXX}
           -D CMAKE_Fortran_COMPILER=${FC}
-          -D CMAKE_CXX_FLAGS="${CXXFLAGS} ${{ matrix.EXTRA_CXX_FLAGS }}"
+          -D CMAKE_CXX_FLAGS="${CXX_FLAGS}"
           -D OVERRIDE_ARCH=x86-64
+          -D ENABLE_WARNINGS=${ENABLE_WARNINGS:-'ON'}
           -D CHARM_ROOT=${MATRIX_CHARM_ROOT:-$CHARM_ROOT}
           -D CMAKE_BUILD_TYPE=${{ matrix.build_type }}
           -D DEBUG_SYMBOLS=OFF
@@ -723,6 +753,22 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
         # We currently don't require codecov in our guidelines, so don't fail
         # the CI build if codecov fails to upload
         continue-on-error: true
+      # Test only a few unit tests with nvcc for now because compiling the full
+      # suite is too slow and exceed the available memory. We should extend this
+      # to the full suite once we have a better solution for the memory issue.
+      - name: Test nvcc support
+        if: matrix.CUDA == 'ON'
+        working-directory: build
+        run: |
+          make -j${NUMBER_OF_CORES} \
+            Test_DataStructures \
+            Test_Spectral \
+            Test_Utilities \
+            EvolveScalarWave3D
+          ./bin/Test_DataStructures
+          ./bin/Test_Spectral
+          ./bin/Test_Utilities
+          ctest -R PlaneWave3D.yaml --output-on-failure
       # Avoid running out of disk space by cleaning up the build directory
       - name: Clean up unit tests
         working-directory: build
@@ -803,7 +849,10 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
         #   compiler version.
         # - We make sure to use the same compiler flags as the full build above
         #   so ccache is able to speed up the build.
-        if: matrix.build_type == 'Debug' && matrix.ASAN != 'ON'
+        if: >
+          matrix.build_type == 'Debug'
+            && matrix.ASAN != 'ON'
+            && matrix.test_executables != 'OFF'
         working-directory: build
         run: >
           make EvolveBurgers -j${NUMBER_OF_CORES}
@@ -828,6 +877,9 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
           mkdir build-formaline;
           cd build-formaline
 
+          ENABLE_WARNINGS=${{ matrix.ENABLE_WARNINGS }}
+          WERROR="${{ matrix.WERROR != 'OFF' && '-Werror' || '' }}"
+          CXX_FLAGS="${WERROR} ${{ matrix.EXTRA_CXX_FLAGS }}"
           BUILD_PYTHON_BINDINGS=${{ matrix.BUILD_PYTHON_BINDINGS }}
           MATRIX_CHARM_ROOT=${{ matrix.CHARM_ROOT }}
           MEMORY_ALLOCATOR=${{ matrix.MEMORY_ALLOCATOR }};
@@ -838,8 +890,9 @@ ${{ matrix.build_type }}-pch-${{ matrix.use_pch || 'ON' }}"
           -D CMAKE_C_COMPILER=${CC}
           -D CMAKE_CXX_COMPILER=${CXX}
           -D CMAKE_Fortran_COMPILER=${FC}
-          -D CMAKE_CXX_FLAGS="${CXXFLAGS} ${{ matrix.EXTRA_CXX_FLAGS }}"
+          -D CMAKE_CXX_FLAGS="${CXX_FLAGS}"
           -D OVERRIDE_ARCH=x86-64
+          -D ENABLE_WARNINGS=${ENABLE_WARNINGS:-'ON'}
           -D BUILD_SHARED_LIBS=ON
           -D CHARM_ROOT=${MATRIX_CHARM_ROOT:-$CHARM_ROOT}
           -D CMAKE_BUILD_TYPE=${{ matrix.build_type }}

cmake/AddCxxFlag.cmake

@@ -1,6 +1,12 @@
 # Distributed under the MIT License.
 # See LICENSE.txt for details.
 
+# Have to compile a file (can be empty) to check for flags.
+# Kokkos' nvcc_wrapper doesn't support taking the file as stdin, so we
+# have to write it to a file.
+set(_CHECK_CXX_FLAGS_SOURCE "${CMAKE_BINARY_DIR}/CMakeFiles/CheckCxxFlags.cpp")
+write_file(${_CHECK_CXX_FLAGS_SOURCE} "")
+
 # Checks if a flag is supported by the compiler and creates the target
 # TARGET_NAME whose INTERFACE_COMPILE_OPTIONS are set to the FLAG_TO_CHECK
 # - LANGUAGE: language to check, setting the compiler and generated property
@@ -12,18 +18,22 @@ function(create_compile_flag_target LANGUAGE XTYPE FLAG_TO_CHECK TARGET_NAME)
   # In order to check for a -Wno-* flag in gcc, you have to check the
   # -W* version instead.  See http://gcc.gnu.org/wiki/FAQ#wnowarning
   string(REGEX REPLACE ^-Wno- -W POSITIVE_FLAG_TO_CHECK ${FLAG_TO_CHECK})
+  # Escape quotes for compiler command
+  string(REPLACE "\"" "\\\"" POSITIVE_FLAG_TO_CHECK ${POSITIVE_FLAG_TO_CHECK})
   execute_process(
     COMMAND
     bash -c
     "LC_ALL=POSIX ${CMAKE_${LANGUAGE}_COMPILER} -Werror \
-${POSITIVE_FLAG_TO_CHECK} -x ${XTYPE} -c - <<< \"\" -o /dev/null"
+${POSITIVE_FLAG_TO_CHECK} -x ${XTYPE} \
+-c ${_CHECK_CXX_FLAGS_SOURCE} -o /dev/null"
     RESULT_VARIABLE RESULT
     ERROR_VARIABLE ERROR_FROM_COMPILATION
     OUTPUT_QUIET)
   if(NOT TARGET ${TARGET_NAME})
     add_library(${TARGET_NAME} INTERFACE)
   endif(NOT TARGET ${TARGET_NAME})
   if(${RESULT} EQUAL 0)
+    string(REPLACE " " ";" FLAG_TO_CHECK ${FLAG_TO_CHECK})
     set_property(TARGET ${TARGET_NAME}
       APPEND PROPERTY
       INTERFACE_COMPILE_OPTIONS
@@ -69,7 +79,8 @@ function(create_compile_flags_target LANGUAGE XTYPE FLAGS_TO_CHECK TARGET_NAME)
     COMMAND
     bash -c
     "LC_ALL=POSIX ${CMAKE_${LANGUAGE}_COMPILER} -Werror \
-${POSITIVE_FLAGS_WITH_SPACES} -x ${XTYPE} -c - <<< \"\" -o /dev/null"
+${POSITIVE_FLAGS_WITH_SPACES} -x ${XTYPE} \
+-c ${_CHECK_CXX_FLAGS_SOURCE} -o /dev/null"
     RESULT_VARIABLE RESULT
     ERROR_VARIABLE ERROR_FROM_COMPILATION
     OUTPUT_QUIET)

cmake/EnableWarnings.cmake

@@ -26,12 +26,6 @@ if(${ENABLE_WARNINGS})
 -Wmissing-include-dirs;\
 -Wmissing-noreturn;\
 -Wnewline-eof;\
--Wno-dangling-reference;\
--Wno-documentation-unknown-command;\
--Wno-mismatched-tags;\
--Wno-non-template-friend;\
--Wno-type-limits;\
--Wno-undefined-var-template;\
 -Wnon-virtual-dtor;\
 -Wold-style-cast;\
 -Woverloaded-virtual;\
@@ -44,18 +38,26 @@ if(${ENABLE_WARNINGS})
 -Wstack-protector;\
 -Wswitch-default;\
 -Wunreachable-code;\
--Wno-gnu-zero-variadic-macro-arguments;\
 -Wwrite-strings" SpectreWarnings)
+else()
+  add_library(SpectreWarnings INTERFACE)
 endif()
 
-# GCC 7.1ish and newer warn about noexcept changing mangled names,
-# but we don't care
-create_cxx_flag_target("-Wno-noexcept-type" SpectreWarnNoNoexceptType)
-
+# Disable some warnings
+create_cxx_flags_target(
+    "-Wno-dangling-reference;\
+-Wno-documentation-unknown-command;\
+-Wno-mismatched-tags;\
+-Wno-non-template-friend;\
+-Wno-type-limits;\
+-Wno-undefined-var-template;\
+-Wno-gnu-zero-variadic-macro-arguments;\
+-Wno-noexcept-type"
+  SpectreDisableSomeWarnings)
 target_link_libraries(
   SpectreWarnings
   INTERFACE
-  SpectreWarnNoNoexceptType
+  SpectreDisableSomeWarnings
   )
 
 # GCC versions below 13 don't respect 'GCC diagnostic' pragmas to disable
@@ -72,6 +74,16 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU"
     )
 endif()
 
+# Suppress CUDA warnings that we don't want
+create_cxx_flag_target(
+  "-Xcudafe \"--diag_suppress=177,186,191,554,1301,1305,2189,3060,20012\""
+  SpectreCudaWarnings)
+target_link_libraries(
+  SpectreWarnings
+  INTERFACE
+  SpectreCudaWarnings
+  )
+
 target_link_libraries(
   SpectreFlags
   INTERFACE

cmake/SetupBoost.cmake

@@ -108,6 +108,17 @@ set_property(TARGET Boost::boost
   INTERFACE_COMPILE_DEFINITIONS
   $<$<COMPILE_LANGUAGE:CXX>:BOOST_ALLOW_DEPRECATED_HEADERS>)
 
+# Old versions of boost only enabled variadic macros for known compilers.
+# This changed in boost 1.75.0, where variadic macros are always enabled.
+# We enable this here manually for older versions of boost so the code compiles
+# with nvcc.
+if(Boost_VERSION VERSION_LESS 1.75.0)
+  set_property(TARGET Boost::boost
+    APPEND PROPERTY
+    INTERFACE_COMPILE_DEFINITIONS
+    $<$<COMPILE_LANGUAGE:CXX>:BOOST_PP_VARIADICS=1>)
+endif()
+
 # Override the boost index type to match the STL for Boost.MultiArray
 # (std::ptrdiff_t to std::size_t)
 # Note: This header guard changed in Boost 1.73.0

cmake/SetupKokkos.cmake

@@ -23,6 +23,11 @@ if(SPECTRE_KOKKOS)
     find_package(CUDAToolkit REQUIRED)
     set(Kokkos_ENABLE_CUDA_LAMBDA ON CACHE BOOL
       "Enable lambda expressions in CUDA")
+    # Allow constexpr functions to be called from device code
+    # without the need for a device annotation.
+    # See https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#constexpr-functions-and-function-templates
+    set(Kokkos_ENABLE_CUDA_CONSTEXPR ON CACHE BOOL
+      "Enable constexpr in CUDA")
   endif()
 
   find_package(Kokkos)
@@ -42,29 +47,18 @@ if(SPECTRE_KOKKOS)
     )
     FetchContent_MakeAvailable(Kokkos)
   endif()
-
-  if (TARGET Kokkos::kokkos
-      AND Kokkos_ENABLE_CUDA
-      AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
-    set_property(TARGET Kokkos::kokkos
-      APPEND PROPERTY
-      INTERFACE_COMPILE_OPTIONS
-      $<$<COMPILE_LANGUAGE:CXX>:
-      -Xcudafe;"--diag_suppress=186,191,554,1301,1305,2189,3060">
-    )
-  endif()
-
-  if (TARGET Kokkos::kokkos
-      AND Kokkos_ENABLE_CUDA
-      AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-    if (${CUDAToolkit_VERSION_MAJOR}.${CUDAToolkit_VERSION_MINOR}
-        VERSION_GREATER 12.0)
-      message(STATUS "CUDA 12.1 is only partially supported by Clang")
-      set_property(TARGET Kokkos::kokkos
-        APPEND PROPERTY
-        INTERFACE_COMPILE_OPTIONS
-        $<$<COMPILE_LANGUAGE:CXX>:-Wno-unknown-cuda-version>
-      )
-    endif()
+else()
+  # We don't have Kokkos enabled, so we fall back to checking manually if the
+  # compiler is NVIDIA's nvcc.
+  execute_process(
+    COMMAND ${CMAKE_CXX_COMPILER} --version
+    OUTPUT_VARIABLE _COMPILER_VERSION
+    OUTPUT_STRIP_TRAILING_WHITESPACE)
+  string(REPLACE "\n" " " _COMPILER_VERSION ${_COMPILER_VERSION})
+  string(FIND ${_COMPILER_VERSION} "nvcc" _COMPILER_IS_NVCC)
+  if(${_COMPILER_IS_NVCC} GREATER -1)
+    set(KOKKOS_CXX_COMPILER_ID "NVIDIA")
+  else()
+    set(KOKKOS_CXX_COMPILER_ID ${CMAKE_CXX_COMPILER_ID})
   endif()
 endif()

docs/DevGuide/Brigand.md

@@ -813,7 +813,7 @@ predicate checks that the element's `value` is not equal to zero.
 
 \note
 The predicate must return the same true value for each element for `all` to
-return true.
+return true. This behaviour may differ by compiler, so it can't be relied upon.
 \snippet Test_TMPLDocumentation.cpp tmpl::all:inhomogeneous
 
 \see \ref any "tmpl::any", \ref none "tmpl::none"
@@ -836,7 +836,7 @@ expensive predicates.
 
 \note
 The predicate must return the same false value for each element for `any` to
-return false.
+return false. This behaviour may differ by compiler, so it can't be relied upon.
 \snippet Test_TMPLDocumentation.cpp tmpl::any:inhomogeneous
 
 \see \ref all "tmpl::all", \ref found "tmpl::found", \ref none "tmpl::none"
@@ -963,7 +963,7 @@ See \ref any "tmpl::any" for discussion.
 
 \note
 The predicate must return the same false value for each element for `none` to
-return true.
+return true. This behaviour may differ by compiler, so it can't be relied upon.
 \snippet Test_TMPLDocumentation.cpp tmpl::none:inhomogeneous
 
 \see \ref all "tmpl::all", \ref any "tmpl::any", \ref not_found

src/DataStructures/DataBox/DataBox.hpp

@@ -34,7 +34,6 @@
 #include "Utilities/ForceInline.hpp"
 #include "Utilities/Gsl.hpp"
 #include "Utilities/NoSuchType.hpp"
-#include "Utilities/Overloader.hpp"
 #include "Utilities/PrintHelpers.hpp"
 #include "Utilities/Requires.hpp"
 #include "Utilities/Serialization/Serialize.hpp"

src/Domain/Creators/TimeDependentOptions/BinaryCompactObject.cpp

@@ -92,8 +92,8 @@ TimeDependentMapOptions<IsCylindrical>::create_worldtube_functions_of_time()
       std::make_unique<FunctionsOfTime::IntegratedFunctionOfTime>(
           initial_time_,
           std::array<double, 2>{
-              {{gsl::at(expansion_map_options_.value().initial_values, 0)},
-               {gsl::at(expansion_map_options_.value().initial_values, 1)}}},
+              {gsl::at(expansion_map_options_.value().initial_values, 0),
+               gsl::at(expansion_map_options_.value().initial_values, 1)}},
           initial_expiration_time, false);
   result[expansion_outer_boundary_name] =
       std::make_unique<FunctionsOfTime::FixedSpeedCubic>(