Merge pull request #1300 from KrisThielemans/GHA_CUDA

- Add CUDA build to GitHub Actions - small CMake clean-up - fix numerical rounding problem in `test_interpolate_projdata` (appeared in gcc-12)
UCL · Feb 5, 2024 · b486fc8 · b486fc8
2 parents 3329bd7 + 8ca178c
commit b486fc8
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 16 deletions.
diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml
@@ -41,6 +41,7 @@ jobs:
         - os: ubuntu-latest
           compiler: gcc
           compiler_version: 9
+          cuda_version: "0"
           BUILD_FLAGS: "-DSTIR_OPENMP=ON"
           BUILD_TYPE: "Release"
           parallelproj: "ON"
@@ -49,6 +50,7 @@ jobs:
         - os: ubuntu-latest
           compiler: clang
           #compiler_version:
+          cuda_version: "0"
           BUILD_FLAGS: "-DSTIR_OPENMP=ON"
           BUILD_TYPE: "Release"
           parallelproj: "ON"
@@ -58,6 +60,7 @@ jobs:
         - os: ubuntu-latest
           compiler: gcc
           compiler_version: 10
+          cuda_version: "0"
           BUILD_FLAGS: "-DSTIR_OPENMP=ON -DCMAKE_CXX_STANDARD=17"
           BUILD_TYPE: "Debug"
           parallelproj: "OFF"
@@ -66,14 +69,25 @@ jobs:
         - os: ubuntu-latest
           compiler: gcc
           compiler_version: 12
+          cuda_version: "0"
           BUILD_FLAGS: "-DSTIR_OPENMP=ON -DCMAKE_CXX_STANDARD=14"
           BUILD_TYPE: "RelWithDebInfo"
           parallelproj: "ON"
           ROOT: "OFF"
           ITK: "ON"
+        - os: ubuntu-latest
+          compiler: gcc
+          compiler_version: 12
+          cuda_version: "12.1.0"
+          BUILD_FLAGS: "-DSTIR_OPENMP=ON -DCMAKE_CXX_STANDARD=14"
+          BUILD_TYPE: "Release"
+          parallelproj: "ON"
+          ROOT: "OFF"
+          ITK: "ON"
         - os: macOS-latest
           compiler: gcc
           compiler_version: 11
+          cuda_version: "0"
           BUILD_FLAGS: "-DSTIR_OPENMP=OFF"
           parallelproj: "OFF"
           BUILD_TYPE: "Debug"
@@ -92,7 +106,7 @@ jobs:
       # let's run all of them, as opposed to aborting when one fails
       fail-fast: false
 
-    name: ${{ matrix.os }}-${{ matrix.compiler }}${{ matrix.compiler_version }}-${{ matrix.BUILD_TYPE }}-pp=${{ matrix.parallelproj }}-ROOT=${{ matrix.ROOT }}
+    name: ${{ matrix.os }}-${{ matrix.compiler }}${{ matrix.compiler_version }}-cuda${{ matrix.cuda_version }}-${{ matrix.BUILD_TYPE }}-pp=${{ matrix.parallelproj }}-ROOT=${{ matrix.ROOT }}
 
     steps:
     - name: disk space
@@ -168,6 +182,13 @@ jobs:
           echo CC="$CC" >> $GITHUB_ENV
           echo CXX="$CXX" >> $GITHUB_ENV
 
+    - if: matrix.cuda_version != '0'
+      uses: Jimver/[email protected]
+      id: cuda-toolkit
+      with:
+          cuda: ${{ matrix.cuda_version }}
+          linux-local-args: '["--toolkit"]'
+
     - name: install_dependencies
       shell: bash
       run: |
@@ -228,7 +249,10 @@ jobs:
              git clone --depth 1 --branch v1.3.7 https://github.com/gschramm/parallelproj
              mkdir parallelproj/build
              cd parallelproj/build
-             cmake .. -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX} -DSKIP_CUDA_LIB:BOOL=ON
+             if test "${{matrix.cuda_version}}" == "0"; then
+               extra_args="-DSKIP_CUDA_LIB:BOOL=ON"
+             fi
+             cmake .. -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX} ${extra_args}
              cmake --build . --target install --config Release
              cd ../..
           fi
@@ -259,7 +283,7 @@ jobs:
     - name: ccache
       uses: hendrikmuhs/ccache-action@v1
       with:
-        key: ${{ matrix.os }}-${{ matrix.compiler }}-${{ matrix.compiler_version }}-${{ matrix.BUILD_TYPE }}
+        key: ${{ matrix.os }}-${{ matrix.compiler }}${{ matrix.compiler_version }}-cuda${{ matrix.cuda_version }}-${{ matrix.BUILD_TYPE }}
         max-size: "2G"
 
     - name: configure
@@ -311,11 +335,23 @@ jobs:
       env:
           BUILD_TYPE: ${{ matrix.BUILD_TYPE }}
       run: |
+          set -vx
           cd ${GITHUB_WORKSPACE}/build
+          # find test exclusions. Complicated due to need of single -E argument and | pattern
+          if test ${{matrix.cuda_version}} != "0"; then
+              # No CUDA drivers on GitHub Actions
+              EXCLUDE="parallelproj|test_blocks_on_cylindrical_projectors"
+          fi
           # don't run all of them in Debug mode as it takes too long
           if test ${BUILD_TYPE} = Debug; then
-              EXCLUDE="-E test_data_processor_projectors|test_export_array|test_ArcCorrection"
+              EXCLUDE_Debug="test_data_processor_projectors|test_export_array|test_ArcCorrection"
+              EXCLUDE="${EXCLUDE_Debug}${EXCLUDE:+"|"}${EXCLUDE}"
+          fi
+          # prepend -E
+          if [ ! -z "${EXCLUDE}" ]; then
+              EXCLUDE="-E ${EXCLUDE}"
           fi
+          # execute tests
           ctest --output-on-failure -C ${BUILD_TYPE} ${EXCLUDE}
 
     - name: recon_test_pack
@@ -346,9 +382,8 @@ jobs:
       uses: actions/upload-artifact@v3
       if: failure()
       with:
-        name: recon_test_pack_log_files-${{ matrix.os }}-${{ matrix.compiler }}${{ matrix.compiler_version }}-${{ matrix.BUILD_TYPE }}-pp=${{ matrix.parallelproj }}-ROOT=${{ matrix.ROOT }}
-        path: |
-          ${{ github.workspace }}/recon_test_pack/**/*.log
+        name: recon_test_pack_log_files-${{ matrix.os }}-${{ matrix.compiler }}${{ matrix.compiler_version }}-cuda${{ matrix.cuda_version }}-${{ matrix.BUILD_TYPE }}-pp=${{ matrix.parallelproj }}-ROOT=${{ matrix.ROOT }}
+        path: ${{ github.workspace }}/recon_test_pack/**/*.log
           ${{ github.workspace }}/recon_test_pack/**/my_*v
           ${{ github.workspace }}/recon_test_pack/**/my_*s
         retention-days: 7

diff --git a/src/listmode_utilities/CMakeLists.txt b/src/listmode_utilities/CMakeLists.txt
@@ -35,8 +35,8 @@ if (HAVE_ECAT)
 endif()
 
 if (STIR_WITH_NiftyPET_PROJECTOR)
-  list(APPEND ${dir_EXE_SOURCES} conv_NiftyPET_stir)
-  list(APPEND ${dir_EXE_SOURCES} lm_to_projdata_NiftyPET)
+  list(APPEND ${dir_EXE_SOURCES} conv_NiftyPET_stir.cxx)
+  list(APPEND ${dir_EXE_SOURCES} lm_to_projdata_NiftyPET.cxx)
 endif()
 
 

diff --git a/src/test/NiftyPET_projector/CMakeLists.txt b/src/test/NiftyPET_projector/CMakeLists.txt
@@ -14,10 +14,10 @@ set(dir_INVOLVED_TEST_EXE_SOURCES ${dir}_INVOLVED_TEST_EXE_SOURCES)
 set(dir_SIMPLE_TEST_EXE_SOURCES_NO_REGISTRIES ${dir}_SIMPLE_TEST_EXE_SOURCES_NO_REGISTRIES)
 
 set(${dir_INVOLVED_TEST_EXE_SOURCES}
-	test_ProjectorNiftyPET_adjoint
+	test_ProjectorNiftyPET_adjoint.cxx
 )
 
 ADD_TEST(test_ProjectorNiftyPET_adjoint
    ${CMAKE_CURRENT_BINARY_DIR}/test_ProjectorNiftyPET_adjoint 3)
 
-include(stir_test_exe_targets)
+include(stir_test_exe_targets)
diff --git a/src/test/test_interpolate_projdata.cxx b/src/test/test_interpolate_projdata.cxx
@@ -2,6 +2,7 @@
 //
 /*
   Copyright 2023, Positrigo AG, Zurich
+  Copyright 2024, University College London
   This file is part of STIR.
 
   SPDX-License-Identifier: Apache-2.0
@@ -188,6 +189,16 @@ void InterpolationTests::compare_segment_shape(const SegmentBySinogram<float>& s
   check_if_equal(sumVoxelsOutsideMask, 0U, "there were non-zero voxels outside the masked area");
 }
 
+static void make_symmetric_object(VoxelsOnCartesianGrid<float>& emission_map)
+{
+  const float z_voxel_size = emission_map.get_grid_spacing()[1];
+  const float z_centre = (emission_map.get_min_z() + emission_map.get_max_z())/2.F * z_voxel_size;
+  // choose a length that isn't exactly equal to a number of planes (or half), as that
+  // is sensitive to rounding error
+  auto cylinder = EllipsoidalCylinder(z_voxel_size*4.5, 80, 80, CartesianCoordinate3D<float>(z_centre, 0, 0));
+  cylinder.construct_volume(emission_map, CartesianCoordinate3D<int>(1, 1, 1));
+}
+
 void InterpolationTests::scatter_interpolation_test_blocks()
 {
   info("Performing symmetric interpolation test for BlocksOnCylindrical scanner");
@@ -213,8 +224,7 @@ void InterpolationTests::scatter_interpolation_test_blocks()
 
   // define a cylinder precisely in the middle of the FOV, such that symmetry can be used for validation
   auto emission_map = VoxelsOnCartesianGrid<float>(*downsampled_proj_data_info, 1);
-  auto cylinder = EllipsoidalCylinder(41, 80, 80, CartesianCoordinate3D<float>(emission_map.get_max_z()/2 * emission_map.get_grid_spacing()[1], 0, 0));
-  cylinder.construct_volume(emission_map, CartesianCoordinate3D<int>(1, 1, 1));
+  make_symmetric_object(emission_map);
   write_to_file("downsampled_cylinder_map", emission_map);
 
   // project the cylinder onto the downsampled scanner proj data
@@ -266,8 +276,8 @@ void InterpolationTests::scatter_interpolation_test_cyl()
 
   // define a cylinder precisely in the middle of the FOV, such that symmetry can be used for validation
   auto emission_map = VoxelsOnCartesianGrid<float>(*downsampled_proj_data_info, 1);
-  auto cylinder = EllipsoidalCylinder(40, 80, 80, CartesianCoordinate3D<float>(emission_map.get_max_z()/2 * emission_map.get_grid_spacing()[1], 0, 0));
-  cylinder.construct_volume(emission_map, CartesianCoordinate3D<int>(1, 1, 1));
+  make_symmetric_object(emission_map);
+  write_to_file("downsampled_cylinder_map_cyl", emission_map);
 
   // project the cylinder onto the downsampled scanner proj data
   auto pm = ProjMatrixByBinUsingRayTracing();
@@ -384,7 +394,7 @@ void InterpolationTests::scatter_interpolation_test_cyl_asymmetric()
   auto proj_data = ProjDataInMemory(std::make_shared<ExamInfo>(exam_info), proj_data_info);
   auto downsampled_proj_data = ProjDataInMemory(std::make_shared<ExamInfo>(exam_info), downsampled_proj_data_info);
 
-  // define a cylinder precisely in the middle of the FOV, such that symmetry can be used for validation
+  // define asymetric object
   auto emission_map = VoxelsOnCartesianGrid<float>(*proj_data_info, 1);
   auto cyl_map = VoxelsOnCartesianGrid<float>(*proj_data_info, 1);
   auto cylinder = EllipsoidalCylinder(40, 40, 20, CartesianCoordinate3D<float>(90, 100, 0));