fix: Remove det(cov)<=0 check from HelicalTrackLinearizer (#2127)

IMO this check should be removed as `det(cov)<=0` should never be true anyways and replacing it with a different cov will not improve things. I logged into this using `full_chain_odd.py` and it looks like `det(cov)<=0` will fire a bunch of times because of numerical issues and the backup cov will also have `det(cov)<=0`
acts-project · May 17, 2023 · 9d5d41b · 9d5d41b
1 parent 39ff1e8
commit 9d5d41b
Show file tree

Hide file tree

Showing 12 changed files with 23 additions and 14 deletions.
diff --git a/CI/physmon/reference/performance_amvf_orthogonal_hist.root b/CI/physmon/reference/performance_amvf_orthogonal_hist.root
diff --git a/CI/physmon/reference/performance_amvf_seeded_hist.root b/CI/physmon/reference/performance_amvf_seeded_hist.root
diff --git a/CI/physmon/reference/performance_amvf_truth_estimated_hist.root b/CI/physmon/reference/performance_amvf_truth_estimated_hist.root
diff --git a/CI/physmon/reference/performance_amvf_truth_smeared_hist.root b/CI/physmon/reference/performance_amvf_truth_smeared_hist.root
diff --git a/CI/physmon/reference/performance_ivf_orthogonal_hist.root b/CI/physmon/reference/performance_ivf_orthogonal_hist.root
diff --git a/CI/physmon/reference/performance_ivf_seeded_hist.root b/CI/physmon/reference/performance_ivf_seeded_hist.root
diff --git a/CI/physmon/reference/performance_ivf_truth_estimated_hist.root b/CI/physmon/reference/performance_ivf_truth_estimated_hist.root
diff --git a/CI/physmon/reference/performance_ivf_truth_smeared_hist.root b/CI/physmon/reference/performance_ivf_truth_smeared_hist.root
diff --git a/Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp b/Core/include/Acts/Vertexing/HelicalTrackLinearizer.ipp
@@ -48,16 +48,6 @@ Acts::Result<Acts::LinearizedTrack> Acts::
   }
   BoundSymMatrix parCovarianceAtPCA = endParams.covariance().value();
 
-  if (parCovarianceAtPCA.determinant() <= 0) {
-    // Use the original parameters
-    paramsAtPCA = params.parameters();
-    auto pos = endParams.position(gctx);
-    positionAtPCA[ePos0] = pos[ePos0];
-    positionAtPCA[ePos1] = pos[ePos1];
-    positionAtPCA[ePos2] = pos[ePos2];
-    parCovarianceAtPCA = params.covariance().value();
-  }
-
   // phiV and functions
   double phiV = paramsAtPCA(BoundIndices::eBoundPhi);
   double sinPhiV = std::sin(phiV);
@@ -120,7 +110,7 @@ Acts::Result<Acts::LinearizedTrack> Acts::
   predParamsAtPCA[2] = phiAtPCA;
   predParamsAtPCA[3] = th;
   predParamsAtPCA[4] = qOvP;
-  predParamsAtPCA[5] = 0.;
+  predParamsAtPCA[5] = positionAtPCA[eTime];
 
   // Fill position jacobian (D_k matrix), Eq. 5.36 in Ref(1)
   ActsMatrix<eBoundSize, 4> positionJacobian;

diff --git a/Examples/Algorithms/Vertexing/src/AdaptiveMultiVertexFinderAlgorithm.cpp b/Examples/Algorithms/Vertexing/src/AdaptiveMultiVertexFinderAlgorithm.cpp
@@ -100,6 +100,7 @@ ActsExamples::AdaptiveMultiVertexFinderAlgorithm::execute(
   // We do not want to use a beamspot constraint here
   finderConfig.useBeamSpotConstraint = false;
   finderConfig.tracksMaxZinterval = 1. * Acts::UnitConstants::mm;
+  finderConfig.maxIterations = 200;
 
   // Instantiate the finder
   Finder finder(finderConfig, logger().cloneWithSuffix("AMVFinder"));
@@ -109,6 +110,15 @@ ActsExamples::AdaptiveMultiVertexFinderAlgorithm::execute(
   auto [inputTrackParameters, inputTrackPointers] =
       makeParameterContainers(ctx, m_inputTrackParameters, m_inputTrajectories);
 
+  for (const auto& trk : inputTrackParameters) {
+    if (trk.covariance() && trk.covariance()->determinant() <= 0) {
+      // actually we should consider this as an error but I do not want the CI
+      // to fail
+      ACTS_WARNING("input track " << trk << " has det(cov) = "
+                                  << trk.covariance()->determinant());
+    }
+  }
+
   //////////////////////////////////////////////
   /* Full tutorial example code for reference */
   //////////////////////////////////////////////

diff --git a/Examples/Algorithms/Vertexing/src/IterativeVertexFinderAlgorithm.cpp b/Examples/Algorithms/Vertexing/src/IterativeVertexFinderAlgorithm.cpp
@@ -69,6 +69,15 @@ ActsExamples::ProcessCode ActsExamples::IterativeVertexFinderAlgorithm::execute(
   auto [inputTrackParameters, inputTrackPointers] =
       makeParameterContainers(ctx, m_inputTrackParameters, m_inputTrajectories);
 
+  for (const auto& trk : inputTrackParameters) {
+    if (trk.covariance() && trk.covariance()->determinant() <= 0) {
+      // actually we should consider this as an error but I do not want the CI
+      // to fail
+      ACTS_WARNING("input track " << trk << " has det(cov) = "
+                                  << trk.covariance()->determinant());
+    }
+  }
+
   // Set up EigenStepper
   Acts::EigenStepper<> stepper(m_cfg.bField);
 

diff --git a/Examples/Python/tests/test_examples.py b/Examples/Python/tests/test_examples.py
@@ -1092,7 +1092,7 @@ def test_full_chain_odd_example(tmp_path):
     )
     assert script.exists()
     env = os.environ.copy()
-    env["ACTS_LOG_FAILURE_THRESHOLD"] = "WARNING"
+    env["ACTS_LOG_FAILURE_THRESHOLD"] = "ERROR"
     try:
         subprocess.check_call(
             [sys.executable, str(script), "-n1"],
@@ -1124,7 +1124,7 @@ def test_full_chain_odd_example_pythia_geant4(tmp_path):
     )
     assert script.exists()
     env = os.environ.copy()
-    env["ACTS_LOG_FAILURE_THRESHOLD"] = "WARNING"
+    env["ACTS_LOG_FAILURE_THRESHOLD"] = "ERROR"
     try:
         stdout = subprocess.check_output(
             [sys.executable, str(script), "-n1", "--geant4", "--ttbar"],
@@ -1168,7 +1168,7 @@ def test_ML_Ambiguity_Solver(tmp_path, assert_root_hash):
     )
     assert script.exists()
     env = os.environ.copy()
-    env["ACTS_LOG_FAILURE_THRESHOLD"] = "WARNING"
+    env["ACTS_LOG_FAILURE_THRESHOLD"] = "ERROR"
     try:
         subprocess.check_call(
             [sys.executable, str(script), "-n5", "--MLSolver"],