Fix and rework Kálmán filter p and pT cuts

This commit fixes the $p$ and $p_T$ cuts in the Kálmán filter, which
were previously entirely broken. The CLI options could not correctly
distinguish between the $p$ and $p_T$ cut, a distinction which is
entirely unnecessary in the first place. This commit splits the cut up
so that both can be supported at the same time, and sets a sensible
default for the $p_T$ cut.

Author: stephenswat

core/include/traccc/finding/details/combinatorial_kalman_filter.hpp

@@ -507,11 +507,8 @@ combinatorial_kalman_filter(
             // Update the actor config
             s4.min_step_length = config.min_step_length_for_next_surface;
             s4.max_count = config.max_step_counts_for_next_surface;
-            if (config.is_min_pT) {
-                s3.min_pT(static_cast<scalar_type>(config.min_p_mag));
-            } else {
-                s3.min_p(static_cast<scalar_type>(config.min_p_mag));
-            }
+            s3.min_pT(static_cast<scalar_type>(config.min_pT));
+            s3.min_p(static_cast<scalar_type>(config.min_p));
 
             // Propagate to the next surface
             propagator.propagate(propagation, detray::tie(s0, s1, s2, s3, s4));

core/include/traccc/finding/finding_config.hpp

@@ -46,8 +46,8 @@ struct finding_config {
     detray::propagation::config propagation{};
 
     /// Minimum momentum for reconstructed tracks
-    bool is_min_pT = true;
-    float min_p_mag = 100.f * traccc::unit<float>::MeV;
+    float min_p = 100.f * traccc::unit<float>::MeV;
+    float min_pT = 600.f * traccc::unit<float>::MeV;
 
     /// Particle hypothesis
     traccc::pdg_particle<traccc::scalar> ptc_hypothesis =
@@ -75,20 +75,6 @@ struct finding_config {
     /// @note This parameter affects GPU-based track finding only.
     unsigned int initial_links_per_seed = 100;
     /// @}
-
-    /// Set the momentum limit to @param p
-    TRACCC_HOST_DEVICE
-    inline void min_p(const float p) {
-        is_min_pT = false;
-        min_p_mag = p;
-    }
-
-    /// Set the transverse momentum limit to @param p
-    TRACCC_HOST_DEVICE
-    inline void min_pT(const float p) {
-        is_min_pT = true;
-        min_p_mag = p;
-    }
 };
 
 }  // namespace traccc

device/common/include/traccc/finding/device/impl/propagate_to_next_surface.ipp

@@ -88,11 +88,8 @@ TRACCC_HOST_DEVICE inline void propagate_to_next_surface(
     typename detray::detail::tuple_element<5, actor_tuple_type>::type::state s5;
     s5.min_step_length = cfg.min_step_length_for_next_surface;
     s5.max_count = cfg.max_step_counts_for_next_surface;
-    if (cfg.is_min_pT) {
-        s4.min_pT(static_cast<scalar_t>(cfg.min_p_mag));
-    } else {
-        s4.min_p(static_cast<scalar_t>(cfg.min_p_mag));
-    }
+    s4.min_pT(static_cast<scalar_t>(cfg.min_pT));
+    s4.min_p(static_cast<scalar_t>(cfg.min_p));
 
     // Propagate to the next surface
     propagator.propagate(propagation, detray::tie(s0, s2, s3, s4, s5));

examples/options/src/track_finding.cpp

@@ -12,6 +12,7 @@
 #include "traccc/utils/particle.hpp"
 
 // System include(s).
+#include <format>
 #include <sstream>
 
 namespace traccc::opts {
@@ -65,11 +66,11 @@ track_finding::track_finding() : interface("Track Finding Options") {
                          "PDG number for the particle hypothesis");
     m_desc.add_options()(
         "min-total-momentum",
-        po::value(&m_config.min_p_mag)->default_value(m_config.min_p_mag),
+        po::value(&m_config.min_p)->default_value(m_config.min_p),
         "Minimum total track momentum [GeV]");
     m_desc.add_options()(
         "min-transverse-momentum",
-        po::value(&m_config.min_p_mag)->default_value(m_config.min_p_mag),
+        po::value(&m_config.min_pT)->default_value(m_config.min_pT),
         "Minimum transverse track momentum [GeV]");
     m_desc.add_options()(
         "duplicate-removal-minimum-length",
@@ -78,12 +79,9 @@ track_finding::track_finding() : interface("Track Finding Options") {
         "Minimum track length for deduplication (0 to disable) [cardinal]");
 }
 
-void track_finding::read(const po::variables_map &vm) {
-
-    m_config.min_p_mag *= traccc::unit<float>::GeV;
-
-    // If not set as total momentum, interpret as transverse momentum
-    m_config.is_min_pT = vm["min-total-momentum"].defaulted();
+void track_finding::read(const po::variables_map &) {
+    m_config.min_p *= traccc::unit<float>::GeV;
+    m_config.min_pT *= traccc::unit<float>::GeV;
 }
 
 track_finding::operator finding_config() const {
@@ -126,16 +124,12 @@ std::unique_ptr<configuration_printable> track_finding::as_printable() const {
         std::to_string(m_config.duplicate_removal_minimum_length)));
     cat->add_child(std::make_unique<configuration_kv_pair>(
         "PDG number", std::to_string(m_pdg_number)));
-    // How to interpret the minimum track momentum value
-    if (m_config.is_min_pT) {
-        cat->add_child(std::make_unique<configuration_kv_pair>(
-            "Minimum transverse track momentum",
-            std::to_string(m_config.min_p_mag)));
-    } else {
-        cat->add_child(std::make_unique<configuration_kv_pair>(
-            "Minimum total track momentum",
-            std::to_string(m_config.min_p_mag)));
-    }
+    cat->add_child(std::make_unique<configuration_kv_pair>(
+        "Minimum pT",
+        std::format("{} GeV", m_config.min_pT / traccc::unit<float>::GeV)));
+    cat->add_child(std::make_unique<configuration_kv_pair>(
+        "Minimum p",
+        std::format("{} GeV", m_config.min_p / traccc::unit<float>::GeV)));
 
     return cat;
 }

tests/cpu/test_ckf_sparse_tracks_telescope.cpp

@@ -127,6 +127,8 @@ TEST_P(CkfSparseTrackTelescopeTests, Run) {
     typename traccc::finding_config cfg;
     cfg.ptc_hypothesis = ptc;
     cfg.chi2_max = 200.f;
+    cfg.min_p = 0;
+    cfg.min_pT = 10.f * unit<float>::MeV;
 
     // Finding algorithm object
     traccc::host::combinatorial_kalman_filter_algorithm host_finding(cfg,