Add some extra assertions (acts-project#971)

Add some assertions for the next candidate index and make sure the track parameters are valid. Also tries to catch potential divisions by zero in the stepper and catch invalid surface/volume descriptors early. This lead to some minor followup corrections in the tests

Author: niermann999

core/include/detray/builders/detector_builder.hpp

@@ -69,6 +69,7 @@ class detector_builder {
     /// functionality
     template <class builder_t>
     DETRAY_HOST auto decorate(dindex volume_idx) -> builder_t* {
+        assert(has_volume(volume_idx));
 
         m_volumes[volume_idx] =
             std::make_unique<builder_t>(std::move(m_volumes[volume_idx]));

core/include/detray/builders/volume_builder.hpp

@@ -211,9 +211,8 @@ class volume_builder : public volume_builder_interface<detector_t> {
         std::size_t n_portals{0u};
         for (auto& sf_desc : m_surfaces) {
 
-            const auto sf = geometry::surface{det, sf_desc};
-
-            sf.template visit_mask<detail::mask_index_update>(sf_desc);
+            det._masks.template visit<detail::mask_index_update>(sf_desc.mask(),
+                                                                 sf_desc);
             sf_desc.set_volume(m_volume.index());
             sf_desc.update_transform(trf_offset);
             sf_desc.set_index(sf_offset++);
@@ -297,21 +296,6 @@ struct mask_index_update {
     }
 };
 
-/// TODO: Remove once the material builder is used everywhere
-/// A functor to update the material index in surface objects
-struct material_index_update {
-
-    template <typename group_t, typename index_t, typename surface_t>
-    DETRAY_HOST inline void operator()(
-        [[maybe_unused]] const group_t& group,
-        [[maybe_unused]] const index_t& /*index*/,
-        [[maybe_unused]] surface_t& sf) const {
-        if constexpr (!concepts::grid<typename group_t::value_type>) {
-            sf.update_material(static_cast<dindex>(group.size()));
-        }
-    }
-};
-
 }  // namespace detail
 
 }  // namespace detray

core/include/detray/geometry/surface.hpp

@@ -53,7 +53,11 @@ class surface {
     /// @param desc from that detector.
     DETRAY_HOST_DEVICE
     constexpr surface(const detector_t &det, const descr_t &desc)
-        : m_detector{det}, m_desc{desc} {}
+        : m_detector{det}, m_desc{desc} {
+        assert(!m_desc.barcode().is_invalid());
+        assert(m_desc.index() < det.surfaces().size());
+        assert(m_desc.transform() < det.transform_store().size());
+    }
 
     /// Constructor from detector @param det and barcode @param bcd in
     /// that detector.
@@ -84,20 +88,30 @@ class surface {
     /// @returns the surface barcode
     DETRAY_HOST_DEVICE
     constexpr auto barcode() const -> geometry::barcode {
+        assert(!m_desc.barcode().is_invalid());
         return m_desc.barcode();
     }
 
     /// @returns the index of the mother volume
     DETRAY_HOST_DEVICE
-    constexpr auto volume() const -> dindex { return barcode().volume(); }
+    constexpr auto volume() const -> dindex {
+        assert(barcode().volume() < m_detector.volumes().size());
+        return barcode().volume();
+    }
 
     /// @returns the index of the surface in the detector surface lookup
     DETRAY_HOST_DEVICE
-    constexpr auto index() const -> dindex { return barcode().index(); }
+    constexpr auto index() const -> dindex {
+        assert(barcode().index() < m_detector.surfaces().size());
+        return barcode().index();
+    }
 
     /// @returns the surface id (sensitive, passive or portal)
     DETRAY_HOST_DEVICE
-    constexpr auto id() const -> surface_id { return barcode().id(); }
+    constexpr auto id() const -> surface_id {
+        assert(barcode().id() != surface_id::e_unknown);
+        return barcode().id();
+    }
 
     /// @returns the extra bits in the barcode
     DETRAY_HOST_DEVICE
@@ -110,7 +124,7 @@ class surface {
     /// @returns the surface source link
     DETRAY_HOST_DEVICE
     constexpr auto source() const {
-        return m_detector.surface(m_desc.barcode()).source;
+        return m_detector.surface(barcode()).source;
     }
 
     /// @returns true if the surface is a senstive detector module.
@@ -156,6 +170,7 @@ class surface {
     DETRAY_HOST_DEVICE
     constexpr auto transform(const context &ctx) const
         -> const transform3_type & {
+        assert(m_desc.transform() < m_detector.transform_store().size());
         return m_detector.transform_store().at(m_desc.transform(), ctx);
     }
 
@@ -239,8 +254,8 @@ class surface {
     /// @tparam Args      types of additional arguments to the functor
     template <typename functor_t, typename... Args>
     DETRAY_HOST_DEVICE constexpr auto visit_mask(Args &&... args) const {
+        assert(!m_desc.mask().is_invalid());
         const auto &masks = m_detector.mask_store();
-
         return masks.template visit<functor_t>(m_desc.mask(),
                                                std::forward<Args>(args)...);
     }
@@ -251,8 +266,8 @@ class surface {
     /// @tparam Args      types of additional arguments to the functor
     template <typename functor_t, typename... Args>
     DETRAY_HOST_DEVICE constexpr auto visit_material(Args &&... args) const {
+        assert(has_material());
         const auto &materials = m_detector.material_store();
-
         return materials.template visit<functor_t>(m_desc.material(),
                                                    std::forward<Args>(args)...);
     }

core/include/detray/geometry/tracking_volume.hpp

@@ -57,7 +57,10 @@ class tracking_volume {
     /// Constructor from detector @param det and volume descriptor
     /// @param vol_idx from that detector.
     constexpr tracking_volume(const detector_t &det, const descr_t &desc)
-        : m_detector{det}, m_desc{desc} {}
+        : m_detector{det}, m_desc{desc} {
+        assert(m_desc.index() < det.volumes().size());
+        assert(m_desc.id() != volume_id::e_unknown);
+    }
 
     /// Constructor from detector @param det and volume index @param vol_idx in
     /// that detector.
@@ -168,8 +171,8 @@ class tracking_volume {
     /// @tparam Args      types of additional arguments to the functor
     template <typename functor_t, typename... Args>
     DETRAY_HOST_DEVICE constexpr auto visit_material(Args &&... args) const {
+        assert(has_material());
         const auto &materials = m_detector.material_store();
-
         return materials.template visit<functor_t>(m_desc.material(),
                                                    std::forward<Args>(args)...);
     }

core/include/detray/navigation/direct_navigator.hpp

@@ -306,6 +306,7 @@ class direct_navigator {
                                         const context_type &ctx) const {
         // Do not resurrect a failed/finished navigation state
         assert(navigation.status() > navigation::status::e_on_target);
+        assert(!track.is_invalid());
 
         if (navigation.is_complete()) {
             navigation.m_heartbeat = false;
@@ -323,6 +324,8 @@ class direct_navigator {
         const context_type &ctx = {},
         const bool is_before_actor_run = true) const {
 
+        assert(!track.is_invalid());
+
         if (navigation.is_complete()) {
             navigation.m_heartbeat = false;
             return true;

core/include/detray/navigation/navigator.hpp

@@ -223,20 +223,23 @@ class navigator {
         DETRAY_HOST_DEVICE
         inline auto target() const -> const candidate_t & {
             assert(!is_exhausted());
+            assert(m_next >= 0);
             return m_candidates[static_cast<std::size_t>(m_next)];
         }
 
         /// @returns last valid candidate (by position in the cache) - const
         DETRAY_HOST_DEVICE
         inline auto last() const -> const candidate_t & {
             assert(!is_exhausted());
+            assert(m_next >= 0);
             return m_candidates[static_cast<std::size_t>(m_last)];
         }
 
         /// Scalar representation of the navigation state,
         /// @returns distance to next
         DETRAY_HOST_DEVICE
         scalar_type operator()() const {
+            assert(math::isfinite(target().path));
             return static_cast<scalar_type>(direction()) * target().path;
         }
 
@@ -573,6 +576,7 @@ class navigator {
         DETRAY_HOST_DEVICE
         inline void set_next(dindex pos) {
             m_next = pos;
+            assert(m_next >= 0);
             assert(m_next <= m_last + 1);
             assert(m_next < static_cast<dist_t>(k_cache_capacity) + 1);
         }
@@ -582,6 +586,8 @@ class navigator {
         inline void set_next(candidate_itr_t new_next) {
             m_next = static_cast<dist_t>(
                 detray::ranges::distance(m_candidates.begin(), new_next));
+            assert(m_next >= 0);
+            assert(m_next <= m_last + 1);
             assert(m_next < static_cast<dist_t>(k_cache_capacity));
         }
 
@@ -718,6 +724,7 @@ class navigator {
 
         // Do not resurrect a failed/finished navigation state
         assert(navigation.status() > navigation::status::e_on_target);
+        assert(!track.is_invalid());
 
         // Clean up state
         navigation.clear();
@@ -777,6 +784,8 @@ class navigator {
         const context_type &ctx = {},
         const bool /*is_before_actor*/ = true) const {
 
+        assert(!track.is_invalid());
+
         // Candidates are re-evaluated based on the current trust level.
         // Should result in 'full trust'
         bool is_init = update_kernel(track, navigation, cfg, ctx);

core/include/detray/propagator/actors/parameter_resetter.hpp

@@ -35,6 +35,7 @@ struct parameter_resetter : actor {
         const auto sf = navigation.get_surface();
         stepping() = sf.bound_to_free_vector(propagation._context,
                                              stepping.bound_params());
+        assert(!stepping().is_invalid());
 
         // Reset jacobian transport to identity matrix
         stepping.reset_transport_jacobian();

core/include/detray/propagator/actors/parameter_transporter.hpp

@@ -111,6 +111,8 @@ struct parameter_transporter : actor {
 
         // Set surface link
         bound_params.set_surface_link(sf.barcode());
+
+        assert(!bound_params.is_invalid());
     }
 
     template <typename propagator_state_t>

core/include/detray/propagator/actors/pointwise_material_interactor.hpp

@@ -185,6 +185,8 @@ struct pointwise_material_interactor : actor {
                     interactor_state.projected_scattering_angle);
             }
         }
+
+        assert(!bound_params.is_invalid());
     }
 
     /// @brief Update the q over p of bound track parameter

core/include/detray/propagator/base_stepper.hpp

@@ -74,6 +74,8 @@ class base_stepper {
 
             // An invalid barcode - should not be used
             m_bound_params.set_surface_link(geometry::barcode{});
+
+            assert(!m_bound_params.is_invalid());
         }
 
         /// Sets track parameters from bound track parameter.
@@ -84,11 +86,23 @@ class base_stepper {
             const typename detector_t::geometry_context &ctx)
             : m_bound_params(bound_params) {
 
+            assert(!m_bound_params.is_invalid());
+            assert(!m_bound_params.surface_link().is_invalid());
+
             // Departure surface
             const auto sf = tracking_surface{det, bound_params.surface_link()};
 
             // Set free track parameters for stepping/navigation
             m_track = sf.bound_to_free_vector(ctx, bound_params);
+
+            assert(!m_track.is_invalid());
+
+            // Bad seed: track direction points back at the IP
+            // (only test high mom. tracks, as low mom. tracks can be looping)
+            assert(math::fabs(m_track.qop()) > 0.5f ||
+                   vector::dot(m_track.pos(), m_track.dir()) > 0.f ||
+                   algebra::approx_equal(vector::norm(m_track.pos()),
+                                         scalar_type{0.f}, scalar_type{1e-5f}));
         }
 
         /// @returns free track parameters - non-const access