Improve testing and documentation for VecGeom (#1639)

* Loosen hepmc3 tolerance

* Add geometry 'inside' check to status executor

* Do not gather step info for errored tracks

* Use bib reference for dormand prince

* Use expressions instead of shell

* Always save cache if build succeeds

* Make transitive geometry dependencies public

* Unify safety tolerance

.github/workflows/build-spack.yml

@@ -91,18 +91,19 @@ jobs:
         run: |
           sed -e 's/cxxstd=default/cxxstd=${{env.CXXSTD}}/' \
             scripts/ci/spack.yaml > spack.yaml
-          if [ "${{matrix.geometry}}" == "vecgeom" ]; then
+          if ${{matrix.geometry == 'vecgeom'}}; then
             spack -e . add vecgeom+gdml
           fi
-          if ${{(matrix.special != 'minimal'
-                 && matrix.special != 'asanlite'
-                 && env.CXXSTD == '20')
-                && 'true' || 'false'}}; then
+          if ${{(matrix.special != 'minimal')
+                && (matrix.special != 'asanlite')
+                && (env.CXXSTD == '20')}}; then
             spack -e . add root
           fi
-          if [ "${{matrix.geant}}" != "" ]; then
+          if ${{matrix.geant != ''}}; then
             spack -e . add geant4@${{matrix.geant}}
-            spack -e . add [email protected]:
+          fi
+          if ${{matrix.geometry == 'vecgeom' && matrix.geant}}; then
+            spack -e . add [email protected]:
           fi
           spack -vd -e . compiler find --mixed-toolchain
           # Add the spack ref so that updating spack will reconcretize
@@ -136,8 +137,8 @@ jobs:
           echo "${SPACK_VIEW}/bin" >> $GITHUB_PATH
           echo "CMAKE_PREFIX_PATH=${SPACK_VIEW}:${CMAKE_PREFIX_PATH}" >> $GITHUB_ENV
           spack env activate . --sh > "${SPACK_VIEW}/rc"
-      - name: Cache ccache
-        uses: actions/cache@v4
+      - name: Restore ccache
+        uses: actions/cache/restore@v4
         with:
           path: ${{env.CCACHE_DIR}}
           key: ccache-${{env.CMAKE_PRESET}}-${{matrix.geant}}-${{github.run_id}}
@@ -152,7 +153,7 @@ jobs:
           # NOTE: tags have issues, see https://github.com/actions/checkout/issues/2041
           git fetch --tags
           ln -fs scripts/cmake-presets/ci-ubuntu-github.json CMakeUserPresets.json
-          if [ "${{matrix.geant}}" == "11.0" ]; then
+          if ${{matrix.geant == '11.0'}}; then
             # Test overriding of Geant4 environment variables
             . ${SPACK_VIEW}/rc
             test -n "${G4LEDATA}"
@@ -164,7 +165,7 @@ jobs:
         env:
           BASE_REF: "${{format('{0}', github.base_ref || 'develop')}}"
         run: |
-          if [ "${{github.event_name}}" == "schedule" ]; then
+          if ${{github.event_name == 'schedule'}}; then
             echo "Full clang-tidy check on scheduled run."
             ninja -Cbuild -k0
             exit $?
@@ -194,7 +195,13 @@ jobs:
         if: ${{matrix.special != 'clang-tidy'}}
         working-directory: build
         run: |
-            ninja -v -k0
+          ninja -v -k0
+      - name: Save ccache
+        if: ${{always() && steps.build.outcome == 'success'}}
+        uses: actions/cache/save@v4
+        with:
+          path: ${{env.CCACHE_DIR}}
+          key: ccache-${{env.CMAKE_PRESET}}-${{matrix.geant}}-${{github.run_id}}
       - name: Regenerate ROOT test data
         if: ${{matrix.geant == '11.0'}}
         working-directory: build
@@ -205,7 +212,7 @@ jobs:
         if: ${{matrix.special != 'clang-tidy'}}
         continue-on-error: ${{fromJSON(matrix.geant || '0') < 11}} # TODO: fix failing tests
         run: |
-          if [ "${{matrix.geant}}" == "11.0" ]; then
+          if ${{matrix.geant == '11.0'}}; then
             # Note this is ignored for geant4, float, clhep
             export CELER_TEST_STRICT=1
           fi

app/celer-geo/CMakeLists.txt

@@ -15,13 +15,6 @@ set(LIBRARIES
   Celeritas::orange
   nlohmann_json::nlohmann_json
 )
-if(CELERITAS_USE_VecGeom)
-  list(APPEND LIBRARIES VecGeom::vecgeom)
-endif()
-if(CELERITAS_USE_Geant4)
-  celeritas_get_g4libs(_g4_geo_libs geometry)
-  list(APPEND LIBRARIES ${_g4_geo_libs})
-endif()
 
 celeritas_add_executable(celer-geo ${SOURCES})
 celeritas_target_link_libraries(celer-geo ${LIBRARIES})

doc/_static/zotero.bib

@@ -2055,11 +2055,15 @@ @misc{pokorski-updaterd-2023
 }
 
 @book{press-nr-1992,
-  title = {Numerical recipes in {{C}}: the art of scientific computing},
+  title = {Numerical Recipes in {{C}}: The Art of Scientific Computing},
+  shorttitle = {Numerical Recipes in {{C}}},
   editor = {Press, William H.},
-  edition = {4th ed},
+  year = {1992},
+  edition = {2nd ed},
   publisher = {Cambridge University Press},
-  year = {1992}
+  address = {Cambridge ; New York},
+  isbn = {978-0-521-43108-8 978-0-521-43720-2},
+  lccn = {QA76.73.C15 N865 1992}
 }
 
 @article{quast-statusplans-2022,
@@ -2271,6 +2275,17 @@ @article{sempau-dpmfast-2000
   doi = {10.1088/0031-9155/45/8/315}
 }
 
+@article{shampine-rungekutta-1986,
+  title = {Some {{Practical Runge-Kutta Formulas}}},
+  author = {Shampine, Lawrence F},
+  year = {1986},
+  month = jan,
+  volume = {46},
+  number = {173},
+  pages = {135--150},
+  doi = {10.2307/2008219}
+}
+
 @techreport{si-2019,
   title = {The {{International System}} of {{Units}}},
   author = {{Bureau International des Poids et Mesures}},

src/celeritas/field/DormandPrinceStepper.hh

@@ -21,9 +21,8 @@ namespace celeritas
  * The algorithm, RK5(4)7M and the coefficients have been adapted from
  * J. R. Dormand and P. J. Prince, "A family of embedded Runge-Kutta formulae"
  * Journal Computational and Applied Mathematics, volume 6, no 1 (1980) and
- * the coefficients to locate the mid point are taken from L. F. Shampine,
- * "Some Practical Runge-Kutta Formulas", Mathematics of * Computation,
- * volume 46, number 17, pp 147 (1986).
+ * the coefficients to locate the mid point are taken from
+ * \citet{shampine-rungekutta-1986, https://doi.org/10.2307/2008219}.
  *
  * For a given ordinary differential equation, \f$dy/dx = f(x, y)\f$, the
  * fifth order solution, \f$ y_{n+1} \f$, an embedded fourth order solution,

src/celeritas/track/detail/StatusCheckExecutor.hh

@@ -12,26 +12,41 @@
 #include "celeritas/global/CoreTrackView.hh"
 #include "celeritas/track/SimTrackView.hh"
 
+#if !CELER_DEVICE_COMPILE
+#    include "corecel/io/Logger.hh"
+#    include "celeritas/global/Debug.hh"
+#endif
+
 #include "../StatusCheckData.hh"
 
 namespace celeritas
 {
 namespace detail
 {
 //---------------------------------------------------------------------------//
+#if !CELER_DEVICE_COMPILE
+// Print a track and why it failed.
+#    define CELER_PRINT_TRACK(MSG, TRACK) \
+        CELER_LOG_LOCAL(error)            \
+            << MSG << ": " << ::celeritas::StreamableTrack{TRACK};
+#else
+#    define CELER_PRINT_TRACK(MSG, TRACK)
+#endif
+
 /*!
  * Check that the condition is true, otherwise throw an error/assertion.
  *
  * \note This macro is defined so that the condition is still checked in
- * "release" mode.
+ * "release" mode, and so that checking will work on GPU.
  */
-#define CELER_FAIL_IF(COND, MSG)                                      \
-    do                                                                \
-    {                                                                 \
-        if (CELER_UNLIKELY(!(COND)))                                  \
-        {                                                             \
-            CELER_DEBUG_THROW_(MSG ": '" #COND "' failed", internal); \
-        }                                                             \
+#define CELER_FAIL_IF(COND, MSG)                               \
+    do                                                         \
+    {                                                          \
+        if (CELER_UNLIKELY((COND)))                            \
+        {                                                      \
+            CELER_PRINT_TRACK(MSG, track)                      \
+            CELER_DEBUG_THROW_(MSG ": '" #COND "'", internal); \
+        }                                                      \
     } while (0)
 
 //---------------------------------------------------------------------------//
@@ -68,7 +83,7 @@ CELER_FUNCTION void StatusCheckExecutor::operator()(CoreTrackView const& track)
         && state.order < StepActionOrder::end)
     {
         auto prev_status = state.status[tsid];
-        CELER_FAIL_IF(sim.status() >= prev_status,
+        CELER_FAIL_IF(sim.status() < prev_status,
                       "status was improperly reverted");
     }
     if (state.order >= StepActionOrder::pre
@@ -78,7 +93,7 @@ CELER_FUNCTION void StatusCheckExecutor::operator()(CoreTrackView const& track)
         // stepping loop *or* at the very end (in the case where a track is
         // initialized in-place from a secondary). It should be cleared in
         // pre-step
-        CELER_FAIL_IF(sim.status() != TrackStatus::initializing,
+        CELER_FAIL_IF(sim.status() == TrackStatus::initializing,
                       "status cannot be 'initializing' after pre-step");
     }
     if (sim.status() == TrackStatus::inactive)
@@ -101,21 +116,25 @@ CELER_FUNCTION void StatusCheckExecutor::operator()(CoreTrackView const& track)
          * \todo Change this behavior to be a *tracking cut* rather than lost
          * energy.
          */
-        CELER_FAIL_IF(sim.post_step_action(), "missing post-step action");
+        CELER_FAIL_IF(!sim.post_step_action(), "missing post-step action");
     }
 
     if (sim.status() == TrackStatus::alive)
     {
         // If the track fails during initialization, it won't get an
         // along-step action, so only check this if alive
-        CELER_FAIL_IF(sim.along_step_action(), "missing along-step action");
+        CELER_FAIL_IF(!sim.along_step_action(), "missing along-step action");
+
+        // All 'alive' tracks should be inside the geometry
+        CELER_FAIL_IF(track.make_geo_view().is_outside(),
+                      "track is outside the geometry but still 'alive'");
     }
 
     ActionId const prev_along_step = state.along_step_action[tsid];
     ActionId const next_along_step = sim.along_step_action();
     if (state.order > StepActionOrder::pre && next_along_step)
     {
-        CELER_FAIL_IF(prev_along_step == next_along_step,
+        CELER_FAIL_IF(prev_along_step != next_along_step,
                       "along-step action cannot yet change");
     }
 
@@ -127,10 +146,10 @@ CELER_FUNCTION void StatusCheckExecutor::operator()(CoreTrackView const& track)
         // Check that order is increasing if not an "implicit" action
         auto prev_order = params.orders[prev_post_step];
         auto next_order = params.orders[next_post_step];
-        CELER_FAIL_IF((prev_order == params.implicit_order
-                       || next_order == params.implicit_order
-                       || OrderedAction{prev_order, prev_post_step}
-                              < OrderedAction{next_order, next_post_step}),
+        CELER_FAIL_IF(!(prev_order == params.implicit_order
+                        || next_order == params.implicit_order
+                        || OrderedAction{prev_order, prev_post_step}
+                               < OrderedAction{next_order, next_post_step}),
                       "new post-step action is out of order");
     }
 }

src/celeritas/user/detail/StepGatherExecutor.hh

@@ -46,7 +46,8 @@ StepGatherExecutor<P>::operator()(celeritas::CoreTrackView const& track)
 
     {
         auto const sim = track.make_sim_view();
-        bool inactive = (sim.status() == TrackStatus::inactive);
+        bool inactive = (sim.status() == TrackStatus::inactive
+                         || sim.status() == TrackStatus::errored);
 
         if (P == StepPoint::post)
         {

src/geocel/CMakeLists.txt

@@ -52,13 +52,16 @@ if(CELERITAS_USE_Geant4)
     g4/detail/GeantGeoNavCollection.cc
   )
 
+  celeritas_get_g4libs(_cg4_geo_libs geometry)
   celeritas_get_g4libs(_cg4_libs global geometry materials particles
     persistency intercoms)
   list(APPEND _cg4_libs Celeritas::corecel XercesC::XercesC nlohmann_json::nlohmann_json)
-  celeritas_target_link_libraries(geocel_geant4 PRIVATE ${_cg4_libs})
+  celeritas_target_link_libraries(geocel_geant4
+    PRIVATE ${_cg4_libs} ${_cg4_geo_libs})
 
   list(APPEND SOURCES $<TARGET_OBJECTS:geocel_geant4>)
   list(APPEND PRIVATE_DEPS geocel_geant4)
+  list(APPEND PUBLIC_DEPS ${_cg4_geo_libs})
 endif()
 
 if(CELERITAS_USE_VecGeom)
@@ -76,7 +79,6 @@ if(CELERITAS_USE_VecGeom)
     # This needs to be public because its might be needed
     # to resolve the symbols generate by the `nvcc -dlink` of
     # one of the executable.
-    list(APPEND PUBLIC_DEPS VecGeom::vecgeom)
     list(APPEND SOURCES
       vg/RaytraceImager.cu
       vg/detail/VecgeomSetup.cu
@@ -85,6 +87,7 @@ if(CELERITAS_USE_VecGeom)
     # VecGeom is built with CUDA but Celeritas is not
     list(APPEND PRIVATE_DEPS VecGeom::vecgeomcuda)
   endif()
+  list(APPEND PUBLIC_DEPS VecGeom::vecgeom)
 endif()
 
 #-----------------------------------------------------------------------------#

test/accel/HepMC3PrimaryGenerator.test.cc

@@ -148,7 +148,7 @@ TEST_F(HepMC3PrimaryGeneratorTest, no_vertex)
         999.99998921041, 1000, 999.99998921041, 1000.0000107896,
         999.99998921041, 999.99998921041, 1000, 1000, 999.99998921041,
         1000.0000107896, 1000.0000107896, 999.99998474121,};
-    EXPECT_VEC_SOFT_EQ(expected_energy, result.energy);
+    EXPECT_VEC_NEAR(expected_energy, result.energy, coarse_eps);
     static double const expected_dir[] = {0.51986662883182, -0.42922054653912,
         -0.7385854118893, 0.73395459362461, 0.18726575230281, 0.65287226354916,
         -0.40053358241289, -0.081839341451527, 0.91261994913013,

test/geocel/CMakeLists.txt

@@ -44,7 +44,6 @@ if(CELERITAS_USE_VecGeom)
   list(APPEND SOURCES
     vg/VecgeomTestBase.cc
   )
-  list(APPEND PRIVATE_DEPS VecGeom::vecgeom)
 endif()
 
 celeritas_add_test_library(testcel_geocel ${SOURCES})
@@ -53,7 +52,7 @@ celeritas_target_link_libraries(testcel_geocel
   PUBLIC
     testcel_harness Celeritas::geocel
   PRIVATE
-    testcel_core ${_g4_geo_libs} ${VecGeom_LIBRARIES}
+    testcel_core
 )
 
 #-----------------------------------------------------------------------------#

test/geocel/CmsEeBackDeeGeoTest.cc

@@ -77,6 +77,9 @@ void CmsEeBackDeeGeoTest::test_accessors() const
 //---------------------------------------------------------------------------//
 void CmsEeBackDeeGeoTest::test_trace() const
 {
+    // Surface VecGeom needs lower safety tolerance
+    real_type const safety_tol = test_->safety_tol();
+
     {
         SCOPED_TRACE("+z top");
         auto result = test_->track({50, 0.1, 360.1}, {0, 0, 1});
@@ -89,7 +92,8 @@ void CmsEeBackDeeGeoTest::test_trace() const
         static real_type const expected_distances[] = {5.4, 34.1};
         EXPECT_VEC_SOFT_EQ(expected_distances, result.distances);
         static real_type const expected_hw_safety[] = {0.1, 0.1};
-        EXPECT_VEC_SOFT_EQ(expected_hw_safety, result.halfway_safeties);
+        EXPECT_VEC_NEAR(
+            expected_hw_safety, result.halfway_safeties, safety_tol);
     }
     {
         SCOPED_TRACE("+z bottom");
@@ -104,7 +108,8 @@ void CmsEeBackDeeGeoTest::test_trace() const
         EXPECT_VEC_SOFT_EQ(expected_distances, result.distances);
         static real_type const expected_hw_safety[]
             = {0.099999999999956, 0.099999999999953};
-        EXPECT_VEC_SOFT_EQ(expected_hw_safety, result.halfway_safeties);
+        EXPECT_VEC_NEAR(
+            expected_hw_safety, result.halfway_safeties, safety_tol);
     }
 }