From 523d729deff9956b3d163e4a768d595cd0f440f9 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 24 Mar 2022 14:30:19 +0100 Subject: [PATCH 1/9] use fully qualified name for MaterialUpdateStage --- Core/include/Acts/Material/ISurfaceMaterial.hpp | 2 +- Core/include/Acts/Material/MaterialCollector.hpp | 4 ++-- .../detail/PointwiseMaterialInteraction.hpp | 6 +++--- .../Acts/TrackFinding/CombinatorialKalmanFilter.hpp | 8 ++++---- Core/include/Acts/TrackFitting/KalmanFitter.hpp | 12 ++++++------ .../Material/HomogeneousSurfaceMaterialTests.cpp | 6 +++--- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Core/include/Acts/Material/ISurfaceMaterial.hpp b/Core/include/Acts/Material/ISurfaceMaterial.hpp index 93aa7a925ea..7415f74eddc 100644 --- a/Core/include/Acts/Material/ISurfaceMaterial.hpp +++ b/Core/include/Acts/Material/ISurfaceMaterial.hpp @@ -130,7 +130,7 @@ class ISurfaceMaterial { inline double ISurfaceMaterial::factor(NavigationDirection pDir, MaterialUpdateStage mStage) const { - if (mStage == Acts::fullUpdate) { + if (mStage == Acts::MaterialUpdateStage::fullUpdate) { return 1.; } return (pDir * mStage > 0 ? m_splitFactor : 1. - m_splitFactor); diff --git a/Core/include/Acts/Material/MaterialCollector.hpp b/Core/include/Acts/Material/MaterialCollector.hpp index ee20bb25505..fdaf463b448 100644 --- a/Core/include/Acts/Material/MaterialCollector.hpp +++ b/Core/include/Acts/Material/MaterialCollector.hpp @@ -83,13 +83,13 @@ struct MaterialCollector { ACTS_VERBOSE("Update on start surface: post-update mode."); prepofu = state.navigation.currentSurface->surfaceMaterial()->factor( - state.stepping.navDir, postUpdate); + state.stepping.navDir, MaterialUpdateStage::postUpdate); } else if (state.navigation.targetSurface == state.navigation.currentSurface) { ACTS_VERBOSE("Update on target surface: pre-update mode"); prepofu = state.navigation.currentSurface->surfaceMaterial()->factor( - state.stepping.navDir, preUpdate); + state.stepping.navDir, MaterialUpdateStage::preUpdate); } else { ACTS_VERBOSE("Update while pass through: full mode."); } diff --git a/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp b/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp index 987305d0efc..a9000a0defd 100644 --- a/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp +++ b/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp @@ -89,13 +89,13 @@ struct PointwiseMaterialInteraction { /// @return Boolean statement whether the material is valid template bool evaluateMaterialSlab(const propagator_state_t& state, - MaterialUpdateStage updateStage = fullUpdate) { + MaterialUpdateStage updateStage = MaterialUpdateStage::fullUpdate) { // We are at the start surface if (surface == state.navigation.startSurface) { - updateStage = postUpdate; + updateStage = MaterialUpdateStage::postUpdate; // Or is it the target surface ? } else if (surface == state.navigation.targetSurface) { - updateStage = preUpdate; + updateStage = MaterialUpdateStage::preUpdate; } // Retrieve the material properties diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index f383ba457a1..9f9fbadcf89 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -555,7 +555,7 @@ class CombinatorialKalmanFilter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, preUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::preUpdate); // Bind the transported state to the current surface auto boundStateRes = @@ -626,7 +626,7 @@ class CombinatorialKalmanFilter { << result.activeTips.back().first); } // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, postUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::postUpdate); } else if (surface->associatedDetectorElement() != nullptr || surface->surfaceMaterial() != nullptr) { // No splitting on the surface without source links. Set it to one @@ -703,7 +703,7 @@ class CombinatorialKalmanFilter { } if (surface->surfaceMaterial() != nullptr) { // Update state and stepper with material effects - materialInteractor(surface, state, stepper, fullUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::fullUpdate); } } else { // Neither measurement nor material on surface, this branch is still @@ -974,7 +974,7 @@ class CombinatorialKalmanFilter { template void materialInteractor( const Surface* surface, propagator_state_t& state, stepper_t& stepper, - const MaterialUpdateStage& updateStage = fullUpdate) const { + const MaterialUpdateStage& updateStage = MaterialUpdateStage::fullUpdate) const { const auto& logger = state.options.logger; // Indicator if having material bool hasMaterial = false; diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp index b5da1e91954..af694172e46 100644 --- a/Core/include/Acts/TrackFitting/KalmanFitter.hpp +++ b/Core/include/Acts/TrackFitting/KalmanFitter.hpp @@ -527,7 +527,7 @@ class KalmanFitter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, preUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::preUpdate); // Bind the transported state to the current surface auto res = stepper.boundState(state.stepping, *surface, false); @@ -606,7 +606,7 @@ class KalmanFitter { } // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, postUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::postUpdate); // We count the processed state ++result.processedStates; // Update the number of holes count only when encoutering a @@ -682,7 +682,7 @@ class KalmanFitter { } if (surface->surfaceMaterial() != nullptr) { // Update state and stepper with material effects - materialInteractor(surface, state, stepper, fullUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::fullUpdate); } } return Result::success(); @@ -722,7 +722,7 @@ class KalmanFitter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, preUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::preUpdate); // Bind the transported state to the current surface auto res = stepper.boundState(state.stepping, *surface, false); @@ -794,7 +794,7 @@ class KalmanFitter { trackStateProxy.filteredCovariance(), *surface); // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, postUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::postUpdate); } } else if (surface->associatedDetectorElement() != nullptr || surface->surfaceMaterial() != nullptr) { @@ -837,7 +837,7 @@ class KalmanFitter { template void materialInteractor( const Surface* surface, propagator_state_t& state, stepper_t& stepper, - const MaterialUpdateStage& updateStage = fullUpdate) const { + const MaterialUpdateStage& updateStage = MaterialUpdateStage::fullUpdate) const { const auto& logger = state.options.logger; // Indicator if having material bool hasMaterial = false; diff --git a/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp b/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp index ab3638737b1..3c7d549eab3 100644 --- a/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp +++ b/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp @@ -86,9 +86,9 @@ BOOST_AUTO_TEST_CASE(HomogeneousSurfaceMaterial_access_test) { NavigationDirection fDir = forward; NavigationDirection bDir = backward; - MaterialUpdateStage pre = preUpdate; - MaterialUpdateStage full = fullUpdate; - MaterialUpdateStage post = postUpdate; + MaterialUpdateStage pre = MaterialUpdateStage::preUpdate; + MaterialUpdateStage full = MaterialUpdateStage::fullUpdate; + MaterialUpdateStage post = MaterialUpdateStage::postUpdate; // (a) Forward factor material test BOOST_CHECK_EQUAL(hsmfwd.factor(fDir, full), 1.); From 1b3a94b303a60db7f862ebe45df64cf433fff7af Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 24 Mar 2022 14:33:23 +0100 Subject: [PATCH 2/9] capitalize MaterialUpdateStage --- Core/include/Acts/Definitions/Common.hpp | 12 ++++++------ Core/include/Acts/Material/ISurfaceMaterial.hpp | 2 +- Core/include/Acts/Material/MaterialCollector.hpp | 4 ++-- .../detail/PointwiseMaterialInteraction.hpp | 6 +++--- .../Acts/TrackFinding/CombinatorialKalmanFilter.hpp | 8 ++++---- Core/include/Acts/TrackFitting/KalmanFitter.hpp | 12 ++++++------ .../Material/HomogeneousSurfaceMaterialTests.cpp | 6 +++--- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/Core/include/Acts/Definitions/Common.hpp b/Core/include/Acts/Definitions/Common.hpp index 581eb015c8c..5f15b091a55 100644 --- a/Core/include/Acts/Definitions/Common.hpp +++ b/Core/include/Acts/Definitions/Common.hpp @@ -36,13 +36,13 @@ static constexpr ActsScalar s_curvilinearProjTolerance = 0.999995; enum NavigationDirection : int { backward = -1, forward = 1 }; /// This is a steering enum to tell which material update stage: -/// - preUpdate : update on approach of a surface -/// - fullUpdate : update when passing a surface -/// - postUpdate : update when leaving a surface +/// - PreUpdate : update on approach of a surface +/// - FullUpdate : update when passing a surface +/// - PostUpdate : update when leaving a surface enum MaterialUpdateStage : int { - preUpdate = -1, - fullUpdate = 0, - postUpdate = 1 + PreUpdate = -1, + FullUpdate = 0, + PostUpdate = 1 }; /// @enum NoiseUpdateMode to tell how to deal with noise term in covariance diff --git a/Core/include/Acts/Material/ISurfaceMaterial.hpp b/Core/include/Acts/Material/ISurfaceMaterial.hpp index 7415f74eddc..301631ddf9d 100644 --- a/Core/include/Acts/Material/ISurfaceMaterial.hpp +++ b/Core/include/Acts/Material/ISurfaceMaterial.hpp @@ -130,7 +130,7 @@ class ISurfaceMaterial { inline double ISurfaceMaterial::factor(NavigationDirection pDir, MaterialUpdateStage mStage) const { - if (mStage == Acts::MaterialUpdateStage::fullUpdate) { + if (mStage == Acts::MaterialUpdateStage::FullUpdate) { return 1.; } return (pDir * mStage > 0 ? m_splitFactor : 1. - m_splitFactor); diff --git a/Core/include/Acts/Material/MaterialCollector.hpp b/Core/include/Acts/Material/MaterialCollector.hpp index fdaf463b448..44ff375ea9f 100644 --- a/Core/include/Acts/Material/MaterialCollector.hpp +++ b/Core/include/Acts/Material/MaterialCollector.hpp @@ -83,13 +83,13 @@ struct MaterialCollector { ACTS_VERBOSE("Update on start surface: post-update mode."); prepofu = state.navigation.currentSurface->surfaceMaterial()->factor( - state.stepping.navDir, MaterialUpdateStage::postUpdate); + state.stepping.navDir, MaterialUpdateStage::PostUpdate); } else if (state.navigation.targetSurface == state.navigation.currentSurface) { ACTS_VERBOSE("Update on target surface: pre-update mode"); prepofu = state.navigation.currentSurface->surfaceMaterial()->factor( - state.stepping.navDir, MaterialUpdateStage::preUpdate); + state.stepping.navDir, MaterialUpdateStage::PreUpdate); } else { ACTS_VERBOSE("Update while pass through: full mode."); } diff --git a/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp b/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp index a9000a0defd..9b1994bf0e8 100644 --- a/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp +++ b/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp @@ -89,13 +89,13 @@ struct PointwiseMaterialInteraction { /// @return Boolean statement whether the material is valid template bool evaluateMaterialSlab(const propagator_state_t& state, - MaterialUpdateStage updateStage = MaterialUpdateStage::fullUpdate) { + MaterialUpdateStage updateStage = MaterialUpdateStage::FullUpdate) { // We are at the start surface if (surface == state.navigation.startSurface) { - updateStage = MaterialUpdateStage::postUpdate; + updateStage = MaterialUpdateStage::PostUpdate; // Or is it the target surface ? } else if (surface == state.navigation.targetSurface) { - updateStage = MaterialUpdateStage::preUpdate; + updateStage = MaterialUpdateStage::PreUpdate; } // Retrieve the material properties diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index 9f9fbadcf89..589b9a285d0 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -555,7 +555,7 @@ class CombinatorialKalmanFilter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::preUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::PreUpdate); // Bind the transported state to the current surface auto boundStateRes = @@ -626,7 +626,7 @@ class CombinatorialKalmanFilter { << result.activeTips.back().first); } // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::postUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::PostUpdate); } else if (surface->associatedDetectorElement() != nullptr || surface->surfaceMaterial() != nullptr) { // No splitting on the surface without source links. Set it to one @@ -703,7 +703,7 @@ class CombinatorialKalmanFilter { } if (surface->surfaceMaterial() != nullptr) { // Update state and stepper with material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::fullUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::FullUpdate); } } else { // Neither measurement nor material on surface, this branch is still @@ -974,7 +974,7 @@ class CombinatorialKalmanFilter { template void materialInteractor( const Surface* surface, propagator_state_t& state, stepper_t& stepper, - const MaterialUpdateStage& updateStage = MaterialUpdateStage::fullUpdate) const { + const MaterialUpdateStage& updateStage = MaterialUpdateStage::FullUpdate) const { const auto& logger = state.options.logger; // Indicator if having material bool hasMaterial = false; diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp index af694172e46..cc759f6c7b4 100644 --- a/Core/include/Acts/TrackFitting/KalmanFitter.hpp +++ b/Core/include/Acts/TrackFitting/KalmanFitter.hpp @@ -527,7 +527,7 @@ class KalmanFitter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::preUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::PreUpdate); // Bind the transported state to the current surface auto res = stepper.boundState(state.stepping, *surface, false); @@ -606,7 +606,7 @@ class KalmanFitter { } // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::postUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::PostUpdate); // We count the processed state ++result.processedStates; // Update the number of holes count only when encoutering a @@ -682,7 +682,7 @@ class KalmanFitter { } if (surface->surfaceMaterial() != nullptr) { // Update state and stepper with material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::fullUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::FullUpdate); } } return Result::success(); @@ -722,7 +722,7 @@ class KalmanFitter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::preUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::PreUpdate); // Bind the transported state to the current surface auto res = stepper.boundState(state.stepping, *surface, false); @@ -794,7 +794,7 @@ class KalmanFitter { trackStateProxy.filteredCovariance(), *surface); // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::postUpdate); + materialInteractor(surface, state, stepper, MaterialUpdateStage::PostUpdate); } } else if (surface->associatedDetectorElement() != nullptr || surface->surfaceMaterial() != nullptr) { @@ -837,7 +837,7 @@ class KalmanFitter { template void materialInteractor( const Surface* surface, propagator_state_t& state, stepper_t& stepper, - const MaterialUpdateStage& updateStage = MaterialUpdateStage::fullUpdate) const { + const MaterialUpdateStage& updateStage = MaterialUpdateStage::FullUpdate) const { const auto& logger = state.options.logger; // Indicator if having material bool hasMaterial = false; diff --git a/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp b/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp index 3c7d549eab3..bccbec9b7e2 100644 --- a/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp +++ b/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp @@ -86,9 +86,9 @@ BOOST_AUTO_TEST_CASE(HomogeneousSurfaceMaterial_access_test) { NavigationDirection fDir = forward; NavigationDirection bDir = backward; - MaterialUpdateStage pre = MaterialUpdateStage::preUpdate; - MaterialUpdateStage full = MaterialUpdateStage::fullUpdate; - MaterialUpdateStage post = MaterialUpdateStage::postUpdate; + MaterialUpdateStage pre = MaterialUpdateStage::PreUpdate; + MaterialUpdateStage full = MaterialUpdateStage::FullUpdate; + MaterialUpdateStage post = MaterialUpdateStage::PostUpdate; // (a) Forward factor material test BOOST_CHECK_EQUAL(hsmfwd.factor(fDir, full), 1.); From d40c994c96224b32047207fd40cf2e9737f90887 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 24 Mar 2022 14:46:28 +0100 Subject: [PATCH 3/9] use enum class for MaterialUpdateStage --- Core/CMakeLists.txt | 1 + Core/include/Acts/Definitions/Common.hpp | 13 +++++++++- Core/src/Definitions/CMakeLists.txt | 5 ++++ Core/src/Definitions/Common.cpp | 33 ++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 Core/src/Definitions/CMakeLists.txt create mode 100644 Core/src/Definitions/Common.cpp diff --git a/Core/CMakeLists.txt b/Core/CMakeLists.txt index 67728cc3775..9487af2b479 100644 --- a/Core/CMakeLists.txt +++ b/Core/CMakeLists.txt @@ -52,6 +52,7 @@ install( # target source files are added separately add_subdirectory(src/EventData) add_subdirectory(src/Digitization) +add_subdirectory(src/Definitions) add_subdirectory(src/Geometry) add_subdirectory(src/MagneticField) add_subdirectory(src/Material) diff --git a/Core/include/Acts/Definitions/Common.hpp b/Core/include/Acts/Definitions/Common.hpp index 5f15b091a55..9f59895886d 100644 --- a/Core/include/Acts/Definitions/Common.hpp +++ b/Core/include/Acts/Definitions/Common.hpp @@ -10,6 +10,7 @@ #include "Acts/Definitions/Algebra.hpp" +#include #include namespace Acts { @@ -39,12 +40,22 @@ enum NavigationDirection : int { backward = -1, forward = 1 }; /// - PreUpdate : update on approach of a surface /// - FullUpdate : update when passing a surface /// - PostUpdate : update when leaving a surface -enum MaterialUpdateStage : int { +enum class MaterialUpdateStage : int { PreUpdate = -1, FullUpdate = 0, PostUpdate = 1 }; +std::ostream& operator<<(std::ostream& os, MaterialUpdateStage matUpdate); + +inline constexpr auto operator*(MaterialUpdateStage dir, double value) { + return static_cast>(dir) * value; +} + +inline constexpr auto operator*(double value, MaterialUpdateStage dir) { + return value * static_cast>(dir); +} + /// @enum NoiseUpdateMode to tell how to deal with noise term in covariance /// transport /// - removeNoise: subtract noise term diff --git a/Core/src/Definitions/CMakeLists.txt b/Core/src/Definitions/CMakeLists.txt new file mode 100644 index 00000000000..88343570f85 --- /dev/null +++ b/Core/src/Definitions/CMakeLists.txt @@ -0,0 +1,5 @@ +target_sources( + ActsCore + PRIVATE + Common.cpp +) diff --git a/Core/src/Definitions/Common.cpp b/Core/src/Definitions/Common.cpp new file mode 100644 index 00000000000..b84ad799948 --- /dev/null +++ b/Core/src/Definitions/Common.cpp @@ -0,0 +1,33 @@ +// This file is part of the Acts project. +// +// Copyright (C) 2022 CERN for the benefit of the Acts project +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include "Acts/Definitions/Common.hpp" + +#include + +namespace Acts { + +std::ostream& operator<<(std::ostream& os, MaterialUpdateStage matUpdate) { + switch (matUpdate) { + case MaterialUpdateStage::PreUpdate: + os << "PreUpdate (-1)"; + break; + case MaterialUpdateStage::PostUpdate: + os << "PostUpdate (1)"; + break; + case MaterialUpdateStage::FullUpdate: + os << "FullUpdate (0)"; + break; + default: + assert(false && "Invalid material update stage (shouldn't happen)"); + std::abort(); + } + return os; +} + +} // namespace Acts From d3ba5b1ae650643aa24a12cc57d4e9052341b95d Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 24 Mar 2022 18:10:58 +0100 Subject: [PATCH 4/9] fix operation for navdir change --- Core/include/Acts/Material/ISurfaceMaterial.hpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Core/include/Acts/Material/ISurfaceMaterial.hpp b/Core/include/Acts/Material/ISurfaceMaterial.hpp index 301631ddf9d..711e8463839 100644 --- a/Core/include/Acts/Material/ISurfaceMaterial.hpp +++ b/Core/include/Acts/Material/ISurfaceMaterial.hpp @@ -133,7 +133,11 @@ inline double ISurfaceMaterial::factor(NavigationDirection pDir, if (mStage == Acts::MaterialUpdateStage::FullUpdate) { return 1.; } - return (pDir * mStage > 0 ? m_splitFactor : 1. - m_splitFactor); + return ( + pDir * static_cast>(mStage) > + 0 + ? m_splitFactor + : 1. - m_splitFactor); } inline MaterialSlab ISurfaceMaterial::materialSlab( From 521de84e2d016a780b267e7bc320b4f910e9b685 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Thu, 24 Mar 2022 18:11:08 +0100 Subject: [PATCH 5/9] fix format --- .../detail/PointwiseMaterialInteraction.hpp | 5 +++-- .../CombinatorialKalmanFilter.hpp | 16 +++++++++----- .../Acts/TrackFitting/KalmanFitter.hpp | 22 ++++++++++++------- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp b/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp index 9b1994bf0e8..12e2f165c87 100644 --- a/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp +++ b/Core/include/Acts/Propagator/detail/PointwiseMaterialInteraction.hpp @@ -88,8 +88,9 @@ struct PointwiseMaterialInteraction { /// /// @return Boolean statement whether the material is valid template - bool evaluateMaterialSlab(const propagator_state_t& state, - MaterialUpdateStage updateStage = MaterialUpdateStage::FullUpdate) { + bool evaluateMaterialSlab( + const propagator_state_t& state, + MaterialUpdateStage updateStage = MaterialUpdateStage::FullUpdate) { // We are at the start surface if (surface == state.navigation.startSurface) { updateStage = MaterialUpdateStage::PostUpdate; diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index 589b9a285d0..c9c02caeb57 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -555,7 +555,8 @@ class CombinatorialKalmanFilter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::PreUpdate); + materialInteractor(surface, state, stepper, + MaterialUpdateStage::PreUpdate); // Bind the transported state to the current surface auto boundStateRes = @@ -626,7 +627,8 @@ class CombinatorialKalmanFilter { << result.activeTips.back().first); } // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::PostUpdate); + materialInteractor(surface, state, stepper, + MaterialUpdateStage::PostUpdate); } else if (surface->associatedDetectorElement() != nullptr || surface->surfaceMaterial() != nullptr) { // No splitting on the surface without source links. Set it to one @@ -703,7 +705,8 @@ class CombinatorialKalmanFilter { } if (surface->surfaceMaterial() != nullptr) { // Update state and stepper with material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::FullUpdate); + materialInteractor(surface, state, stepper, + MaterialUpdateStage::FullUpdate); } } else { // Neither measurement nor material on surface, this branch is still @@ -972,9 +975,10 @@ class CombinatorialKalmanFilter { /// @param updateStage The materal update stage /// template - void materialInteractor( - const Surface* surface, propagator_state_t& state, stepper_t& stepper, - const MaterialUpdateStage& updateStage = MaterialUpdateStage::FullUpdate) const { + void materialInteractor(const Surface* surface, propagator_state_t& state, + stepper_t& stepper, + const MaterialUpdateStage& updateStage = + MaterialUpdateStage::FullUpdate) const { const auto& logger = state.options.logger; // Indicator if having material bool hasMaterial = false; diff --git a/Core/include/Acts/TrackFitting/KalmanFitter.hpp b/Core/include/Acts/TrackFitting/KalmanFitter.hpp index cc759f6c7b4..b57a0818f03 100644 --- a/Core/include/Acts/TrackFitting/KalmanFitter.hpp +++ b/Core/include/Acts/TrackFitting/KalmanFitter.hpp @@ -527,7 +527,8 @@ class KalmanFitter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::PreUpdate); + materialInteractor(surface, state, stepper, + MaterialUpdateStage::PreUpdate); // Bind the transported state to the current surface auto res = stepper.boundState(state.stepping, *surface, false); @@ -606,7 +607,8 @@ class KalmanFitter { } // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::PostUpdate); + materialInteractor(surface, state, stepper, + MaterialUpdateStage::PostUpdate); // We count the processed state ++result.processedStates; // Update the number of holes count only when encoutering a @@ -682,7 +684,8 @@ class KalmanFitter { } if (surface->surfaceMaterial() != nullptr) { // Update state and stepper with material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::FullUpdate); + materialInteractor(surface, state, stepper, + MaterialUpdateStage::FullUpdate); } } return Result::success(); @@ -722,7 +725,8 @@ class KalmanFitter { stepper.transportCovarianceToBound(state.stepping, *surface); // Update state and stepper with pre material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::PreUpdate); + materialInteractor(surface, state, stepper, + MaterialUpdateStage::PreUpdate); // Bind the transported state to the current surface auto res = stepper.boundState(state.stepping, *surface, false); @@ -794,7 +798,8 @@ class KalmanFitter { trackStateProxy.filteredCovariance(), *surface); // Update state and stepper with post material effects - materialInteractor(surface, state, stepper, MaterialUpdateStage::PostUpdate); + materialInteractor(surface, state, stepper, + MaterialUpdateStage::PostUpdate); } } else if (surface->associatedDetectorElement() != nullptr || surface->surfaceMaterial() != nullptr) { @@ -835,9 +840,10 @@ class KalmanFitter { /// @param updateStage The materal update stage /// template - void materialInteractor( - const Surface* surface, propagator_state_t& state, stepper_t& stepper, - const MaterialUpdateStage& updateStage = MaterialUpdateStage::FullUpdate) const { + void materialInteractor(const Surface* surface, propagator_state_t& state, + stepper_t& stepper, + const MaterialUpdateStage& updateStage = + MaterialUpdateStage::FullUpdate) const { const auto& logger = state.options.logger; // Indicator if having material bool hasMaterial = false; From fe84a59c464bc4b7badd116bc1e10d6d70a971c5 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Mon, 28 Mar 2022 09:47:14 +0200 Subject: [PATCH 6/9] add a test for ISurfaceMaterial::factor --- Tests/UnitTests/Core/Material/CMakeLists.txt | 1 + .../Core/Material/ISurfaceMaterialTests.cpp | 77 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 Tests/UnitTests/Core/Material/ISurfaceMaterialTests.cpp diff --git a/Tests/UnitTests/Core/Material/CMakeLists.txt b/Tests/UnitTests/Core/Material/CMakeLists.txt index 90f52d03ece..fdc1fd89c7b 100644 --- a/Tests/UnitTests/Core/Material/CMakeLists.txt +++ b/Tests/UnitTests/Core/Material/CMakeLists.txt @@ -15,3 +15,4 @@ add_unittest(ProtoSurfaceMaterial ProtoSurfaceMaterialTests.cpp) add_unittest(ProtoVolumeMaterial ProtoVolumeMaterialTests.cpp) add_unittest(SurfaceMaterialMapper SurfaceMaterialMapperTests.cpp) add_unittest(VolumeMaterialMapper VolumeMaterialMapperTests.cpp) +add_unittest(ISurfaceMaterial ISurfaceMaterialTests.cpp) diff --git a/Tests/UnitTests/Core/Material/ISurfaceMaterialTests.cpp b/Tests/UnitTests/Core/Material/ISurfaceMaterialTests.cpp new file mode 100644 index 00000000000..599542cefcb --- /dev/null +++ b/Tests/UnitTests/Core/Material/ISurfaceMaterialTests.cpp @@ -0,0 +1,77 @@ +// This file is part of the Acts project. +// +// Copyright (C) 2022 CERN for the benefit of the Acts project +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include + +#include "Acts/Definitions/Algebra.hpp" +#include "Acts/Material/ISurfaceMaterial.hpp" + +#include + +namespace Acts { + +namespace Test { + +class SurfaceMaterialStub : public ISurfaceMaterial { + using ISurfaceMaterial::ISurfaceMaterial; + + ISurfaceMaterial& operator*=(double /*scale*/) override { return *this; }; + + const MaterialSlab& materialSlab(const Vector2& /*lp*/) const override { + return m_fullMaterial; + } + + const MaterialSlab& materialSlab(const Vector3& /*gp*/) const override { + return m_fullMaterial; + } + + const MaterialSlab& materialSlab(size_t /*bin0*/, + size_t /*bin1*/) const override { + return m_fullMaterial; + } + + std::ostream& toStream(std::ostream& sl) const override { + sl << "SurfaceMaterialStub"; + return sl; + }; + + MaterialSlab m_fullMaterial{}; +}; + +/// Test the constructors +BOOST_AUTO_TEST_CASE(ISurfaceMaterial_factor_test) { + double splitFactor = 42.0; + SurfaceMaterialStub stub{splitFactor}; + + BOOST_CHECK_EQUAL(stub.factor(NavigationDirection::forward, + MaterialUpdateStage::FullUpdate), + 1.0); + + BOOST_CHECK_EQUAL(stub.factor(NavigationDirection::backward, + MaterialUpdateStage::FullUpdate), + 1.0); + + BOOST_CHECK_EQUAL(stub.factor(NavigationDirection::forward, + MaterialUpdateStage::PostUpdate), + splitFactor); + + BOOST_CHECK_EQUAL(stub.factor(NavigationDirection::backward, + MaterialUpdateStage::PreUpdate), + splitFactor); + + BOOST_CHECK_EQUAL( + stub.factor(NavigationDirection::forward, MaterialUpdateStage::PreUpdate), + 1 - splitFactor); + + BOOST_CHECK_EQUAL(stub.factor(NavigationDirection::backward, + MaterialUpdateStage::PostUpdate), + 1 - splitFactor); +} + +} // namespace Test +} // namespace Acts From 18afe3cd45cc7d205adab6ceacee217989e69010 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Mon, 28 Mar 2022 09:51:46 +0200 Subject: [PATCH 7/9] rewrite ISurfaceMaterial::factor logic --- Core/include/Acts/Material/ISurfaceMaterial.hpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Core/include/Acts/Material/ISurfaceMaterial.hpp b/Core/include/Acts/Material/ISurfaceMaterial.hpp index 711e8463839..e51af8ecfab 100644 --- a/Core/include/Acts/Material/ISurfaceMaterial.hpp +++ b/Core/include/Acts/Material/ISurfaceMaterial.hpp @@ -132,12 +132,13 @@ inline double ISurfaceMaterial::factor(NavigationDirection pDir, MaterialUpdateStage mStage) const { if (mStage == Acts::MaterialUpdateStage::FullUpdate) { return 1.; + } else if (mStage == Acts::MaterialUpdateStage::PreUpdate) { + return pDir == NavigationDirection::backward ? m_splitFactor + : 1 - m_splitFactor; + } else /*if (mStage == Acts::MaterialUpdateStage::PostUpdate)*/ { + return pDir == NavigationDirection::forward ? m_splitFactor + : 1 - m_splitFactor; } - return ( - pDir * static_cast>(mStage) > - 0 - ? m_splitFactor - : 1. - m_splitFactor); } inline MaterialSlab ISurfaceMaterial::materialSlab( From 97eec7303efdbf92354c86b00e2c014baf39969f Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 30 Mar 2022 18:07:48 +0200 Subject: [PATCH 8/9] update after GSF went in --- .../Acts/TrackFitting/detail/GsfActor.hpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp index fbaceab27e4..690a2a659dc 100644 --- a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp +++ b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp @@ -260,9 +260,11 @@ struct GsfActor { if (haveMaterial) { if (haveMeasurement) { - applyMultipleScattering(state, stepper, preUpdate); + applyMultipleScattering(state, stepper, + MaterialUpdateStage::PreUpdate); } else { - applyMultipleScattering(state, stepper, fullUpdate); + applyMultipleScattering(state, stepper, + MaterialUpdateStage::FullUpdate); } } @@ -300,7 +302,8 @@ struct GsfActor { // If we only done preUpdate before, now do postUpdate if (haveMaterial && haveMeasurement) { - applyMultipleScattering(state, stepper, postUpdate); + applyMultipleScattering(state, stepper, + MaterialUpdateStage::PostUpdate); } } } @@ -341,7 +344,7 @@ struct GsfActor { // Evaluate material slab auto slab = surface.surfaceMaterial()->materialSlab( old_bound.position(state.stepping.geoContext), state.stepping.navDir, - MaterialUpdateStage::fullUpdate); + MaterialUpdateStage::FullUpdate); auto pathCorrection = surface.pathCorrection(state.stepping.geoContext, @@ -689,9 +692,10 @@ struct GsfActor { /// Apply the multipe scattering to the state template - void applyMultipleScattering( - propagator_state_t& state, const stepper_t& stepper, - const MaterialUpdateStage& updateStage = fullUpdate) const { + void applyMultipleScattering(propagator_state_t& state, + const stepper_t& stepper, + const MaterialUpdateStage& updateStage = + MaterialUpdateStage::FullUpdate) const { const auto& logger = state.options.logger; const auto& surface = *state.navigation.currentSurface; From de3a83ce03871176d9179d34b7b069213f1e5b38 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 30 Mar 2022 18:09:47 +0200 Subject: [PATCH 9/9] remove arithmetic overload: not needed anymore --- Core/include/Acts/Definitions/Common.hpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Core/include/Acts/Definitions/Common.hpp b/Core/include/Acts/Definitions/Common.hpp index 9f59895886d..5270bf65d51 100644 --- a/Core/include/Acts/Definitions/Common.hpp +++ b/Core/include/Acts/Definitions/Common.hpp @@ -48,14 +48,6 @@ enum class MaterialUpdateStage : int { std::ostream& operator<<(std::ostream& os, MaterialUpdateStage matUpdate); -inline constexpr auto operator*(MaterialUpdateStage dir, double value) { - return static_cast>(dir) * value; -} - -inline constexpr auto operator*(double value, MaterialUpdateStage dir) { - return value * static_cast>(dir); -} - /// @enum NoiseUpdateMode to tell how to deal with noise term in covariance /// transport /// - removeNoise: subtract noise term