From fd862c4ed15771025fa1caa154985927d7ec2240 Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Fri, 4 Oct 2024 14:59:48 +0200 Subject: [PATCH 1/5] copy all trackstate information in GSF combined state creation --- Core/include/Acts/TrackFitting/detail/GsfActor.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp index 92c5529d319..00ca1ad5886 100644 --- a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp +++ b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp @@ -742,7 +742,7 @@ struct GsfActor { result.currentTip = proxy.index(); proxy.setReferenceSurface(surface.getSharedPtr()); - proxy.copyFrom(firstCmpProxy, mask); + proxy.copyFrom(firstCmpProxy, mask, /*only_allocated=*/false); auto [prtMean, prtCov] = mergeGaussianMixture(tmpStates.tips, surface, m_cfg.mergeMethod, From ba532693969619bcf0f53d99e9dfe385e28651e9 Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Mon, 7 Oct 2024 14:08:19 +0200 Subject: [PATCH 2/5] fix trackstate copy and add test --- .../Acts/EventData/TrackStateProxy.hpp | 3 ++- .../Acts/TrackFitting/GaussianSumFitter.hpp | 3 +++ .../Acts/TrackFitting/detail/GsfActor.hpp | 12 +++++------ .../Core/EventData/TrackTestsExtra.cpp | 20 +++++++++++++++++++ 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/Core/include/Acts/EventData/TrackStateProxy.hpp b/Core/include/Acts/EventData/TrackStateProxy.hpp index bed0061ecbf..34de86cd3d7 100644 --- a/Core/include/Acts/EventData/TrackStateProxy.hpp +++ b/Core/include/Acts/EventData/TrackStateProxy.hpp @@ -1043,8 +1043,9 @@ class TrackStateProxy { jacobian() = other.jacobian(); } + // NOTE: we should not check hasCalibrated on this, since it + // may be not yet allocated if (ACTS_CHECK_BIT(mask, PM::Calibrated) && - has() && other.template has()) { allocateCalibrated(other.calibratedSize()); diff --git a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp index efe21b58a60..50d553b63d8 100644 --- a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp +++ b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp @@ -434,6 +434,9 @@ struct GaussianSumFitter { state.unset(TrackStatePropMask::Smoothed); } + assert( + !(state.hasUncalibratedSourceLink() && state.calibratedSize() > 2)); + measurementStatesFinal += static_cast(state.typeFlags().test(MeasurementFlag)); } diff --git a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp index 00ca1ad5886..0832cdc2157 100644 --- a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp +++ b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp @@ -732,16 +732,15 @@ struct GsfActor { const auto isMeasurement = firstCmpProxy.typeFlags().test(MeasurementFlag); - const auto mask = - isMeasurement - ? TrackStatePropMask::Calibrated | TrackStatePropMask::Predicted | - TrackStatePropMask::Filtered | TrackStatePropMask::Smoothed - : TrackStatePropMask::Calibrated | TrackStatePropMask::Predicted; + const auto mask = isMeasurement ? TrackStatePropMask::Calibrated | + TrackStatePropMask::Predicted | + TrackStatePropMask::Filtered | + TrackStatePropMask::Smoothed + : TrackStatePropMask::Predicted; auto proxy = result.fittedStates->makeTrackState(mask, result.currentTip); result.currentTip = proxy.index(); - proxy.setReferenceSurface(surface.getSharedPtr()); proxy.copyFrom(firstCmpProxy, mask, /*only_allocated=*/false); auto [prtMean, prtCov] = @@ -762,7 +761,6 @@ struct GsfActor { proxy.shareFrom(TrackStatePropMask::Predicted, TrackStatePropMask::Filtered); } - } else { assert((result.currentTip != MultiTrajectoryTraits::kInvalid && "tip not valid")); diff --git a/Tests/UnitTests/Core/EventData/TrackTestsExtra.cpp b/Tests/UnitTests/Core/EventData/TrackTestsExtra.cpp index 2c16463ebe1..ca444466216 100644 --- a/Tests/UnitTests/Core/EventData/TrackTestsExtra.cpp +++ b/Tests/UnitTests/Core/EventData/TrackTestsExtra.cpp @@ -447,4 +447,24 @@ BOOST_AUTO_TEST_CASE(ReverseTrackStates) { } } +BOOST_AUTO_TEST_CASE(CopyTrackProxyCalibrated) { + VectorTrackContainer vtc{}; + VectorMultiTrajectory mtj{}; + TrackContainer tc{vtc, mtj}; + + constexpr static std::size_t kMeasurementSize = 3; + + auto track1 = tc.makeTrack(); + auto ts = track1.appendTrackState(TrackStatePropMask::Calibrated); + ts.allocateCalibrated(kMeasurementSize); + ts.calibrated() = Vector3::Ones(); + ts.calibratedCovariance() = SquareMatrix3::Identity(); + ts.setSubspaceIndices(BoundSubspaceIndices{}); + + auto tsCopy = track1.appendTrackState(TrackStatePropMask::Calibrated); + tsCopy.copyFrom(ts, TrackStatePropMask::Calibrated, false); + + BOOST_CHECK_EQUAL(ts.calibratedSize(), tsCopy.calibratedSize()); +} + BOOST_AUTO_TEST_SUITE_END() From 8c2d1349bf2694ff3a29100d4f3b7a7ee93ed08d Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Mon, 7 Oct 2024 14:54:56 +0200 Subject: [PATCH 3/5] make assertion more general --- Core/include/Acts/TrackFitting/GaussianSumFitter.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp index 50d553b63d8..abed191b840 100644 --- a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp +++ b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp @@ -434,8 +434,7 @@ struct GaussianSumFitter { state.unset(TrackStatePropMask::Smoothed); } - assert( - !(state.hasUncalibratedSourceLink() && state.calibratedSize() > 2)); + assert(!state.hasUncalibratedSourceLink() || state.calibratedSize() <= 6); measurementStatesFinal += static_cast(state.typeFlags().test(MeasurementFlag)); From c12458ea38f3b9f80cfda76171bbfb2b00d0c60b Mon Sep 17 00:00:00 2001 From: Benjamin Huth <37871400+benjaminhuth@users.noreply.github.com> Date: Tue, 8 Oct 2024 10:21:24 +0200 Subject: [PATCH 4/5] Update GaussianSumFitter.hpp --- Core/include/Acts/TrackFitting/GaussianSumFitter.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp index abed191b840..efe21b58a60 100644 --- a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp +++ b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp @@ -434,8 +434,6 @@ struct GaussianSumFitter { state.unset(TrackStatePropMask::Smoothed); } - assert(!state.hasUncalibratedSourceLink() || state.calibratedSize() <= 6); - measurementStatesFinal += static_cast(state.typeFlags().test(MeasurementFlag)); } From 71a301096b8342bffe4ee6e93abcd11065f4057c Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Wed, 9 Oct 2024 09:43:57 +0200 Subject: [PATCH 5/5] revert GSF changes, will come in a separate PR --- Core/include/Acts/TrackFitting/detail/GsfActor.hpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp index 0832cdc2157..92c5529d319 100644 --- a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp +++ b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp @@ -732,16 +732,17 @@ struct GsfActor { const auto isMeasurement = firstCmpProxy.typeFlags().test(MeasurementFlag); - const auto mask = isMeasurement ? TrackStatePropMask::Calibrated | - TrackStatePropMask::Predicted | - TrackStatePropMask::Filtered | - TrackStatePropMask::Smoothed - : TrackStatePropMask::Predicted; + const auto mask = + isMeasurement + ? TrackStatePropMask::Calibrated | TrackStatePropMask::Predicted | + TrackStatePropMask::Filtered | TrackStatePropMask::Smoothed + : TrackStatePropMask::Calibrated | TrackStatePropMask::Predicted; auto proxy = result.fittedStates->makeTrackState(mask, result.currentTip); result.currentTip = proxy.index(); - proxy.copyFrom(firstCmpProxy, mask, /*only_allocated=*/false); + proxy.setReferenceSurface(surface.getSharedPtr()); + proxy.copyFrom(firstCmpProxy, mask); auto [prtMean, prtCov] = mergeGaussianMixture(tmpStates.tips, surface, m_cfg.mergeMethod, @@ -761,6 +762,7 @@ struct GsfActor { proxy.shareFrom(TrackStatePropMask::Predicted, TrackStatePropMask::Filtered); } + } else { assert((result.currentTip != MultiTrajectoryTraits::kInvalid && "tip not valid"));