Skip to content

Commit

Permalink
refactor: Use makeX to construct tracks and track states (#3074)
Browse files Browse the repository at this point in the history
Replaces `getX(addX())` with the helper `makeX()`
  • Loading branch information
andiwand committed Apr 4, 2024
1 parent 9c8141e commit 93c3dd0
Show file tree
Hide file tree
Showing 21 changed files with 60 additions and 79 deletions.
2 changes: 1 addition & 1 deletion Core/include/Acts/EventData/TrackProxy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ class TrackProxy {
template <bool RO = ReadOnly, typename = std::enable_if_t<!RO>>
auto appendTrackState(TrackStatePropMask mask = TrackStatePropMask::All) {
auto& tsc = m_container->trackStateContainer();
auto ts = tsc.getTrackState(tsc.addTrackState(mask, tipIndex()));
auto ts = tsc.makeTrackState(mask, tipIndex());
tipIndex() = ts.index();
return ts;
}
Expand Down
19 changes: 8 additions & 11 deletions Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,11 +780,10 @@ class CombinatorialKalmanFilter {
"Create temp track state with mask: " << std::bitset<
sizeof(std::underlying_type_t<TrackStatePropMask>) * 8>(
static_cast<std::underlying_type_t<TrackStatePropMask>>(mask)));
std::size_t tsi = result.stateBuffer->addTrackState(mask, prevTip);
// CAREFUL! This trackstate has a previous index that is not in this
// MultiTrajectory Visiting brackwards from this track state will
// fail!
auto ts = result.stateBuffer->getTrackState(tsi);
auto ts = result.stateBuffer->makeTrackState(mask, prevTip);

if (it == slBegin) {
// only set these for first
Expand Down Expand Up @@ -849,9 +848,8 @@ class CombinatorialKalmanFilter {

// copy this trackstate into fitted states MultiTrajectory
typename traj_t::TrackStateProxy trackState =
result.fittedStates->getTrackState(
result.fittedStates->addTrackState(
mask, candidateTrackState.previous()));
result.fittedStates->makeTrackState(mask,
candidateTrackState.previous());
ACTS_VERBOSE(
"Create SourceLink output track state #"
<< trackState.index() << " with mask: "
Expand Down Expand Up @@ -940,17 +938,16 @@ class CombinatorialKalmanFilter {
result_type& result, bool isSensitive,
std::size_t prevTip) const {
// Add a track state
auto currentTip = result.fittedStates->addTrackState(stateMask, prevTip);
auto trackStateProxy =
result.fittedStates->makeTrackState(stateMask, prevTip);
ACTS_VERBOSE(
"Create "
<< (isSensitive ? "Hole" : "Material") << " output track state #"
<< currentTip << " with mask: "
<< trackStateProxy.index() << " with mask: "
<< std::bitset<sizeof(std::underlying_type_t<TrackStatePropMask>) *
8>{
static_cast<std::underlying_type_t<TrackStatePropMask>>(
stateMask)});
// now get track state proxy back
auto trackStateProxy = result.fittedStates->getTrackState(currentTip);

const auto& [boundParams, jacobian, pathLength] = boundState;
// Fill the track state
Expand Down Expand Up @@ -979,7 +976,7 @@ class CombinatorialKalmanFilter {
trackStateProxy.shareFrom(TrackStatePropMask::Predicted,
TrackStatePropMask::Filtered);

return currentTip;
return trackStateProxy.index();
}

/// @brief CombinatorialKalmanFilter actor operation: material interaction
Expand Down Expand Up @@ -1206,7 +1203,7 @@ class CombinatorialKalmanFilter {
std::vector<typename TrackContainer::TrackProxy> tracks;

for (auto tip : combKalmanResult.lastMeasurementIndices) {
auto track = trackContainer.getTrack(trackContainer.addTrack());
auto track = trackContainer.makeTrack();
track.tipIndex() = tip;

// Set fitted track parameters if available. This will only be the case if
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/TrackFinding/TrackSelector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ void TrackSelector::selectTracks(const input_tracks_t& inputTracks,
if (!isValidTrack(track)) {
continue;
}
auto destProxy = outputTracks.getTrack(outputTracks.addTrack());
auto destProxy = outputTracks.makeTrack();
destProxy.copyFrom(track, false);
destProxy.tipIndex() = track.tipIndex();
}
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/TrackFitting/GaussianSumFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ struct GaussianSumFitter {
return return_error_or_abort(GsfError::NoMeasurementStatesCreatedFinal);
}

auto track = trackContainer.getTrack(trackContainer.addTrack());
auto track = trackContainer.makeTrack();
track.tipIndex() = fwdGsfResult.lastMeasurementTip;

if (options.referenceSurface) {
Expand Down
9 changes: 3 additions & 6 deletions Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,12 +433,9 @@ class Gx2Fitter {

// Add a <mask> TrackState entry multi trajectory. This allocates
// storage for all components, which we will set later.
std::size_t currentTrackIndex =
fittedStates.addTrackState(mask, result.lastTrackIndex);

// now get track state proxy back
typename traj_t::TrackStateProxy trackStateProxy =
fittedStates.getTrackState(currentTrackIndex);
fittedStates.makeTrackState(mask, result.lastTrackIndex);
std::size_t currentTrackIndex = trackStateProxy.index();

// Set the trackStateProxy components with the state from the ongoing
// propagation
Expand Down Expand Up @@ -826,7 +823,7 @@ class Gx2Fitter {
}

// Prepare track for return
auto track = trackContainer.getTrack(trackContainer.addTrack());
auto track = trackContainer.makeTrack();
track.tipIndex() = tipIndex;
track.parameters() = params.parameters();
track.covariance() = fullCovariancePredicted;
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/TrackFitting/KalmanFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,7 @@ class KalmanFitter {
return kalmanResult.result.error();
}

auto track = trackContainer.getTrack(trackContainer.addTrack());
auto track = trackContainer.makeTrack();
track.tipIndex() = kalmanResult.lastMeasurementIndex;
if (kalmanResult.fittedParameters) {
const auto& params = kalmanResult.fittedParameters.value();
Expand Down
5 changes: 2 additions & 3 deletions Core/include/Acts/TrackFitting/detail/GsfActor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -735,9 +735,8 @@ struct GsfActor {
TrackStatePropMask::Filtered | TrackStatePropMask::Smoothed
: TrackStatePropMask::Calibrated | TrackStatePropMask::Predicted;

result.currentTip =
result.fittedStates->addTrackState(mask, result.currentTip);
auto proxy = result.fittedStates->getTrackState(result.currentTip);
auto proxy = result.fittedStates->makeTrackState(mask, result.currentTip);
result.currentTip = proxy.index();

proxy.setReferenceSurface(surface.getSharedPtr());
proxy.copyFrom(firstCmpProxy, mask);
Expand Down
12 changes: 2 additions & 10 deletions Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,8 @@ auto kalmanHandleMeasurement(
// Add a <mask> TrackState entry multi trajectory. This allocates storage for
// all components, which we will set later.
TrackStatePropMask mask = TrackStatePropMask::All;
const std::size_t currentTrackIndex =
fittedStates.addTrackState(mask, lastTrackIndex);

// now get track state proxy back
typename traj_t::TrackStateProxy trackStateProxy =
fittedStates.getTrackState(currentTrackIndex);
fittedStates.makeTrackState(mask, lastTrackIndex);

// Set the trackStateProxy components with the state from the ongoing
// propagation
Expand Down Expand Up @@ -146,12 +142,8 @@ auto kalmanHandleNoMeasurement(
// all components, which we will set later.
TrackStatePropMask mask =
~(TrackStatePropMask::Calibrated | TrackStatePropMask::Filtered);
const std::size_t currentTrackIndex =
fittedStates.addTrackState(mask, lastTrackIndex);

// now get track state proxy back
typename traj_t::TrackStateProxy trackStateProxy =
fittedStates.getTrackState(currentTrackIndex);
fittedStates.makeTrackState(mask, lastTrackIndex);

// Set the trackStateProxy components with the state from the ongoing
// propagation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ ActsExamples::GreedyAmbiguityResolutionAlgorithm::execute(
solvedTracks.ensureDynamicColumns(tracks);

for (auto iTrack : state.selectedTracks) {
auto destProxy = solvedTracks.getTrack(solvedTracks.addTrack());
auto destProxy = solvedTracks.makeTrack();
auto srcProxy = tracks.getTrack(state.trackTips.at(iTrack));
destProxy.copyFrom(srcProxy, false);
destProxy.tipIndex() = srcProxy.tipIndex();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ ActsExamples::ProcessCode ActsExamples::TrackFindingAlgorithm::execute(

if (!m_trackSelector.has_value() ||
m_trackSelector->isValidTrack(track)) {
auto destProxy = tracks.getTrack(tracks.addTrack());
auto destProxy = tracks.makeTrack();
destProxy.copyFrom(track, true); // make sure we copy track states!
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ ActsExamples::AmbiguityResolutionML::prepareOutputTrack(
solvedTracks.ensureDynamicColumns(tracks);

for (auto&& iTrack : goodTracks) {
auto destProxy = solvedTracks.getTrack(solvedTracks.addTrack());
auto destProxy = solvedTracks.makeTrack();
auto srcProxy = tracks.getTrack(iTrack);
destProxy.copyFrom(srcProxy, false);
destProxy.tipIndex() = srcProxy.tipIndex();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ ProcessCode TrackModifier::execute(
outputTracks.ensureDynamicColumns(inputTracks);

for (const auto& inputTrack : inputTracks) {
auto outputTrack = outputTracks.getTrack(outputTracks.addTrack());
auto outputTrack = outputTracks.makeTrack();
outputTrack.copyFrom(inputTrack);
modifyTrack(outputTrack);
}
Expand Down
2 changes: 1 addition & 1 deletion Examples/Io/EDM4hep/src/EDM4hepTrackReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ ProcessCode EDM4hepTrackReader::read(const AlgorithmContext& ctx) {
TrackContainer tracks(trackContainer, trackStateContainer);

for (const auto& inputTrack : trackCollection) {
auto track = tracks.getTrack(tracks.addTrack());
auto track = tracks.makeTrack();
Acts::EDM4hepUtil::readTrack(inputTrack, track, m_cfg.Bz);
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/UnitTests/Core/EventData/MultiTrajectoryTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ BOOST_AUTO_TEST_CASE(MemoryStats) {
}

TestTrackState pc(rng, 2u);
auto ts = mt.getTrackState(mt.addTrackState());
auto ts = mt.makeTrackState();
fillTrackState<VectorMultiTrajectory>(pc, TrackStatePropMask::All, ts);

stats = mt.statistics();
Expand All @@ -251,7 +251,7 @@ BOOST_AUTO_TEST_CASE(Accessors) {
mtj.addColumn<unsigned int>("ndof");
mtj.addColumn<double>("super_chi2");

auto ts = mtj.getTrackState(mtj.addTrackState());
auto ts = mtj.makeTrackState();

ProxyAccessor<unsigned int> ndof("ndof");
ConstProxyAccessor<unsigned int> ndofConst("ndof");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ ACTS_DOES_NOT_COMPILE_SUITE_BEGIN(BuildFromConstRef)
TrackContainer mutTc{mutVtc, mutMtj};
static_assert(!mutTc.ReadOnly, "Unexpectedly read only");

auto t = mutTc.getTrack(mutTc.addTrack());
auto t = mutTc.makeTrack();
t.appendTrackState();
t.appendTrackState();
t.appendTrackState();
t = mutTc.getTrack(mutTc.addTrack());
t = mutTc.makeTrack();
t.appendTrackState();

ConstVectorTrackContainer vtc{std::move(mutVtc)};
Expand Down Expand Up @@ -71,7 +71,7 @@ ACTS_DOES_NOT_COMPILE_SUITE_BEGIN(BuildFromConstRef)
VectorTrackContainer vtc;
VectorMultiTrajectory mtj;
TrackContainer tc{vtc, mtj};
auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();
(void)t;

ConstProxyAccessor<unsigned int> caccNMeasuements("nMeasurements");
Expand Down
30 changes: 14 additions & 16 deletions Tests/UnitTests/Core/EventData/TrackTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,12 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(TrackStateAccess, factory_t, holder_types) {

auto mkts = [&](auto prev) {
if constexpr (std::is_same_v<decltype(prev), IndexType>) {
auto ts =
traj.getTrackState(traj.addTrackState(TrackStatePropMask::All, prev));
auto ts = traj.makeTrackState(TrackStatePropMask::All, prev);
TestTrackState pc(rng, 2u);
fillTrackState<VectorMultiTrajectory>(pc, TrackStatePropMask::All, ts);
return ts;
} else {
auto ts = traj.getTrackState(
traj.addTrackState(TrackStatePropMask::All, prev.index()));
auto ts = traj.makeTrackState(TrackStatePropMask::All, prev.index());
TestTrackState pc(rng, 2u);
fillTrackState<VectorMultiTrajectory>(pc, TrackStatePropMask::All, ts);
return ts;
Expand All @@ -324,7 +322,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(TrackStateAccess, factory_t, holder_types) {
auto ts4 = mkts(ts3);
auto ts5 = mkts(ts4);

auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();
t.tipIndex() = ts5.index();

std::vector<IndexType> act;
Expand All @@ -345,7 +343,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(TrackStateAccess, factory_t, holder_types) {

BOOST_CHECK_EQUAL(t.nTrackStates(), 5);

auto tNone = tc.getTrack(tc.addTrack());
auto tNone = tc.makeTrack();
BOOST_CHECK_EQUAL(tNone.nTrackStates(), 0);

auto tsRange = tNone.trackStatesReversed();
Expand All @@ -364,7 +362,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(TrackIterator, factory_t, holder_types) {
auto& tc = factory.trackContainer();

for (unsigned int i = 0; i < 10; i++) {
auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();
t.tipIndex() = i;
}
BOOST_CHECK_EQUAL(tc.size(), 10);
Expand All @@ -385,7 +383,7 @@ BOOST_AUTO_TEST_CASE(IteratorConcept) {
TrackContainer tc{vtc, mtj};

for (unsigned int i = 0; i < 10; i++) {
auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();
t.tipIndex() = i;
}
BOOST_CHECK_EQUAL(tc.size(), 10);
Expand Down Expand Up @@ -447,7 +445,7 @@ BOOST_AUTO_TEST_CASE(ConstCorrectness) {
TrackContainer tc{vtc, mtj};

for (unsigned int i = 0; i < 10; i++) {
auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();
t.tipIndex() = i;
}

Expand Down Expand Up @@ -487,11 +485,11 @@ BOOST_AUTO_TEST_CASE(BuildFromConstRef) {
TrackContainer mutTc{mutVtc, mutMtj};
static_assert(!mutTc.ReadOnly, "Unexpectedly read only");

auto t = mutTc.getTrack(mutTc.addTrack());
auto t = mutTc.makeTrack();
t.appendTrackState();
t.appendTrackState();
t.appendTrackState();
t = mutTc.getTrack(mutTc.addTrack());
t = mutTc.makeTrack();
t.appendTrackState();

BOOST_CHECK_EQUAL(mutTc.size(), 2);
Expand Down Expand Up @@ -549,7 +547,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(DynamicColumns, factory_t, holder_types) {
tc.template addColumn<float>("col_a");
BOOST_CHECK(tc.hasColumn("col_a"_hash));

auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();
t.template component<float>("col_a") = 5.6f;
BOOST_CHECK_EQUAL((t.template component<float, "col_a"_hash>()), 5.6f);
}
Expand All @@ -575,7 +573,7 @@ BOOST_AUTO_TEST_CASE(EnsureDynamicColumns) {

BOOST_AUTO_TEST_CASE(AppendTrackState) {
TrackContainer tc{VectorTrackContainer{}, VectorMultiTrajectory{}};
auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();

std::vector<VectorMultiTrajectory::TrackStateProxy> trackStates;
trackStates.push_back(t.appendTrackState());
Expand All @@ -596,13 +594,13 @@ BOOST_AUTO_TEST_CASE(ForwardIteration) {
TrackContainer tc{VectorTrackContainer{}, VectorMultiTrajectory{}};
{
// let's create an unrelated track first
auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();
for (std::size_t i = 0; i < 10; i++) {
t.appendTrackState();
}
}

auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();

auto stem = t.appendTrackState();
t.appendTrackState();
Expand Down Expand Up @@ -650,7 +648,7 @@ BOOST_AUTO_TEST_CASE(ForwardIteration) {

BOOST_AUTO_TEST_CASE(CalculateQuantities) {
TrackContainer tc{VectorTrackContainer{}, VectorMultiTrajectory{}};
auto t = tc.getTrack(tc.addTrack());
auto t = tc.makeTrack();

auto ts = t.appendTrackState();
ts.typeFlags().set(MeasurementFlag);
Expand Down

0 comments on commit 93c3dd0

Please sign in to comment.