Skip to content

Commit

Permalink
refactor: GX2F uses TrackContainer + log message cleanup (#1823)
Browse files Browse the repository at this point in the history 
Contrary to what I said before, the GX2F was not yet using `Acts::TrackContainer`. This PR switches it over. 
I also cleaned up some of the log messages in the Chi2 fitter and algorithm, the prefixes there are unnecessary because the loggers already print their own names.
  • Loading branch information
@paulgessinger
paulgessinger authored Feb 3, 2023
1 parent 20a7d0d commit 13f15e1
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 62 deletions.
71 changes: 43 additions & 28 deletions Core/include/Acts/TrackFitting/Chi2Fitter.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ struct Chi2FitterOptions {
template <typename traj_t>
struct Chi2FitterResult {
// Fitted states that the actor has handled.
std::shared_ptr<traj_t> fittedStates;
traj_t* fittedStates{nullptr};

// This is the index of the 'tip' of the track stored in multitrajectory.
// This correspond to the last measurement state in the multitrajectory.
Expand Down Expand Up @@ -294,7 +294,7 @@ class Chi2Fitter {
if (surface != nullptr) {
auto res = processSurface(surface, state, stepper, result);
if (!res.ok()) {
ACTS_ERROR("chi2 | Error in processSurface: " << res.error());
ACTS_ERROR("Error in processSurface: " << res.error());
result.result = res.error();
}
}
Expand All @@ -305,7 +305,7 @@ class Chi2Fitter {
(result.measurementStates > 0 and
state.navigation.navigationBreak)) {
result.missedActiveSurfaces.resize(result.measurementHoles);
ACTS_VERBOSE("chi2 | Finalize...");
ACTS_VERBOSE("Finalize...");
result.finished = true;
}
}
Expand Down Expand Up @@ -350,7 +350,7 @@ class Chi2Fitter {
auto sourcelink_it = inputMeasurements->find(surface->geometryId());
// inputMeasurements is a std::map<GeometryIdentifier, source_link_t>
if (sourcelink_it != inputMeasurements->end()) {
ACTS_VERBOSE("chi2 | processSurface: Measurement surface "
ACTS_VERBOSE(" processSurface: Measurement surface "
<< surface->geometryId() << " detected.");

// add a full TrackState entry multi trajectory
Expand All @@ -376,8 +376,7 @@ class Chi2Fitter {
});

if (!foundExistingSurface) {
ACTS_VERBOSE(
"chi2 | processSurface: Found new surface during update.");
ACTS_VERBOSE(" processSurface: Found new surface during update.");
result.lastTrackIndex = result.fittedStates->addTrackState(
~(TrackStatePropMask::Smoothed | TrackStatePropMask::Filtered),
result.lastTrackIndex);
Expand Down Expand Up @@ -459,7 +458,7 @@ class Chi2Fitter {
typeFlags.set(TrackStateFlag::MeasurementFlag);
++result.measurementStates;
} else {
ACTS_VERBOSE("chi2 | Measurement is determined to be an outlier.");
ACTS_VERBOSE("Measurement is determined to be an outlier.");
typeFlags.set(TrackStateFlag::OutlierFlag);
}

Expand Down Expand Up @@ -621,22 +620,24 @@ class Chi2Fitter {
/// @param end End iterator for the fittable uncalibrated measurements
/// @param sParameters The initial track parameters
/// @param chi2FitterOptions Chi2FitterOptions steering the fit
/// @param trajectory Input trajectory storage to append into
/// @param trackContainer The target track container
/// @note The input measurements are given in the form of @c SourceLink s.
/// It's the calibrators job to turn them into calibrated measurements used in
/// the fit.
///
/// @return the output as an output track
template <typename source_link_iterator_t>
Result<Chi2FitterResult<traj_t>> fit(
source_link_iterator_t it, source_link_iterator_t end,
const BoundTrackParameters& sParameters,
const Chi2FitterOptions<traj_t>& chi2FitterOptions,
std::shared_ptr<traj_t> trajectory = {}) const {
template <typename source_link_iterator_t, typename track_container_t,
template <typename> class holder_t>
auto fit(source_link_iterator_t it, source_link_iterator_t end,
const BoundTrackParameters& sParameters,
const Chi2FitterOptions<traj_t>& chi2FitterOptions,
TrackContainer<track_container_t, traj_t, holder_t>& trackContainer)
const -> Result<typename TrackContainer<track_container_t, traj_t,
holder_t>::TrackProxy> {
// To be able to find measurements later, we put them into a map
// We need to copy input SourceLinks anyways, so the map can own them.
ACTS_VERBOSE("chi2 | preparing " << std::distance(it, end)
<< " input measurements");
ACTS_VERBOSE("preparing " << std::distance(it, end)
<< " input measurements");
std::map<GeometryIdentifier, std::reference_wrapper<const SourceLink>>
inputMeasurements;

Expand All @@ -659,8 +660,6 @@ class Chi2Fitter {
// the result object which will be returned. Overridden every iteration.
Chi2Result c2r;

trajectory = std::make_shared<traj_t>();

BoundTrackParameters vParams = sParameters;
auto updatedStartParameters = sParameters;

Expand All @@ -687,7 +686,7 @@ class Chi2Fitter {
inputResult;

auto& r = inputResult.template get<Chi2FitterResult<traj_t>>();
r.fittedStates = trajectory;
r.fittedStates = &trackContainer.trackStateContainer();
if (i > 0) {
r.lastTrackIndex = c2r.lastTrackIndex;
}
Expand All @@ -696,8 +695,7 @@ class Chi2Fitter {
auto result = m_propagator.template propagate(
updatedStartParameters, propOptions, std::move(inputResult));
if (!result.ok()) {
ACTS_ERROR("chi2 | it=" << i
<< " | propagation failed: " << result.error());
ACTS_ERROR("it=" << i << " | propagation failed: " << result.error());
return result.error();
}

Expand All @@ -711,9 +709,9 @@ class Chi2Fitter {
}

if (!c2rCurrent.result.ok()) {
ACTS_ERROR("chi2 | it=" << i << " | Chi2Fitter failed: "
<< c2rCurrent.result.error() << ", "
<< c2rCurrent.result.error().message());
ACTS_ERROR("it=" << i << " | Chi2Fitter failed: "
<< c2rCurrent.result.error() << ", "
<< c2rCurrent.result.error().message());
return c2rCurrent.result.error();
}

Expand All @@ -730,7 +728,7 @@ class Chi2Fitter {
c2rCurrent.chisquare = c2rCurrent.residuals.transpose() *
c2rCurrent.covariance.inverse() *
c2rCurrent.residuals;
ACTS_VERBOSE("chi2 | it=" << i << " | χ² = " << c2rCurrent.chisquare);
ACTS_VERBOSE("it=" << i << " | χ² = " << c2rCurrent.chisquare);

// copy over data from previous runs (namely chisquares vector)
c2rCurrent.chisquares.reserve(c2r.chisquares.size() + 1);
Expand Down Expand Up @@ -759,8 +757,8 @@ class Chi2Fitter {
c2r.collectorDerive1Chi2Sum);

BoundVector newParamsVec = vParams.parameters() - delta_start_parameters;
ACTS_VERBOSE("chi2 | it=" << i << " | updated parameters = "
<< newParamsVec.transpose());
ACTS_VERBOSE(
"it=" << i << " | updated parameters = " << newParamsVec.transpose());
c2r.fittedParameters =
BoundTrackParameters(vParams.referenceSurface().getSharedPtr(),
newParamsVec, vParams.covariance());
Expand All @@ -769,8 +767,25 @@ class Chi2Fitter {
updatedStartParameters = c2r.fittedParameters.value();
}

auto track = trackContainer.getTrack(trackContainer.addTrack());
track.tipIndex() = c2r.lastMeasurementIndex;
if (c2r.fittedParameters) {
const auto& params = c2r.fittedParameters.value();
track.parameters() = params.parameters();
track.covariance() = params.covariance().value();
track.setReferenceSurface(params.referenceSurface().getSharedPtr());
}

track.nMeasurements() = c2r.measurementStates;
track.nHoles() = c2r.measurementHoles;

if (trackContainer.hasColumn(hashString("chi2"))) {
track.template component<ActsScalar, hashString("chi2")>() =
c2r.chisquare;
}

// Return the converted track
return c2r;
return track;
}
};

Expand Down
17 changes: 10 additions & 7 deletions ...thms/TrackFittingChi2/include/ActsExamples/TrackFittingChi2/TrackFittingChi2Algorithm.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "Acts/EventData/SourceLink.hpp"
#include "Acts/EventData/VectorMultiTrajectory.hpp"
#include "Acts/EventData/VectorTrackContainer.hpp"
#include "Acts/Geometry/TrackingGeometry.hpp"
#include "Acts/TrackFitting/Chi2Fitter.hpp"
#include "ActsExamples/EventData/IndexSourceLink.hpp"
Expand All @@ -35,8 +36,11 @@ class TrackFittingChi2Algorithm final : public BareAlgorithm {
using TrackFitterChi2Options =
Acts::Experimental::Chi2FitterOptions<Acts::VectorMultiTrajectory>;

using TrackFitterChi2Result = Acts::Result<
Acts::Experimental::Chi2FitterResult<Acts::VectorMultiTrajectory>>;
using TrackContainer =
Acts::TrackContainer<Acts::VectorTrackContainer,
Acts::VectorMultiTrajectory, std::shared_ptr>;

using TrackFitterChi2Result = Acts::Result<TrackContainer::TrackProxy>;

/// Fit function that takes the above parameters and runs a fit
/// @note This is separated into a virtual interface to keep compilation units
Expand All @@ -46,8 +50,7 @@ class TrackFittingChi2Algorithm final : public BareAlgorithm {
virtual ~TrackFitterChi2Function() = default;
virtual TrackFitterChi2Result operator()(
const std::vector<Acts::SourceLink>&, const TrackParameters&,
const TrackFitterChi2Options&,
std::shared_ptr<Acts::VectorMultiTrajectory>& trajectory) const = 0;
const TrackFitterChi2Options&, TrackContainer&) const = 0;
};

struct Config {
Expand Down Expand Up @@ -106,7 +109,7 @@ class TrackFittingChi2Algorithm final : public BareAlgorithm {
const Acts::Experimental::Chi2FitterOptions<Acts::VectorMultiTrajectory>&
options,
const std::vector<const Acts::Surface*>& surfSequence,
std::shared_ptr<Acts::VectorMultiTrajectory>& trajectory) const;
TrackContainer& trackContainer) const;

Config m_cfg;
};
Expand All @@ -119,14 +122,14 @@ ActsExamples::TrackFittingChi2Algorithm::fitTrack(
options,
// const Acts::Chi2FitterOptions& options,
const std::vector<const Acts::Surface*>& surfSequence,
std::shared_ptr<Acts::VectorMultiTrajectory>& trajectory) const {
TrackContainer& trackContainer) const {
(void)surfSequence; // TODO: silence unused parameter warning
// if (m_cfg.directNavigation) {
// return (*m_cfg.dFit)(sourceLinks, initialParameters, options,
// surfSequence);
// }

return (*m_cfg.fit)(sourceLinks, initialParameters, options, trajectory);
return (*m_cfg.fit)(sourceLinks, initialParameters, options, trackContainer);
}

} // namespace ActsExamples
51 changes: 29 additions & 22 deletions Examples/Algorithms/TrackFittingChi2/src/TrackFittingChi2Algorithm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

#include "ActsExamples/TrackFittingChi2/TrackFittingChi2Algorithm.hpp"

#include "Acts/EventData/VectorMultiTrajectory.hpp"
#include "Acts/EventData/Track.hpp"
#include "Acts/Surfaces/PerigeeSurface.hpp"
#include "ActsExamples/EventData/ProtoTrack.hpp"
#include "ActsExamples/EventData/Trajectories.hpp"
Expand Down Expand Up @@ -99,10 +99,9 @@ ActsExamples::ProcessCode ActsExamples::TrackFittingChi2Algorithm::execute(
continue;
}

ACTS_VERBOSE("chi2algo | ev="
<< ctx.eventNumber << " | initial parameters: "
<< initialParams.fourPosition(ctx.geoContext).transpose()
<< " -> " << initialParams.unitDirection().transpose());
ACTS_VERBOSE("ev=" << ctx.eventNumber << " | initial parameters: "
<< initialParams.fourPosition(ctx.geoContext).transpose()
<< " -> " << initialParams.unitDirection().transpose());

// Clear & reserve the right size
trackSourceLinks.clear();
Expand All @@ -122,36 +121,44 @@ ActsExamples::ProcessCode ActsExamples::TrackFittingChi2Algorithm::execute(
}
}

ACTS_DEBUG("chi2algo | invoke fitter");
auto mtj = std::make_shared<Acts::VectorMultiTrajectory>();
auto trackContainer = std::make_shared<Acts::VectorTrackContainer>();
auto trackStateContainer = std::make_shared<Acts::VectorMultiTrajectory>();
auto tracks =
std::make_unique<TrackContainer>(trackContainer, trackStateContainer);

tracks->addColumn<Acts::ActsScalar>("chi2");
static Acts::ConstTrackAccessor<Acts::ActsScalar> chisquare{"chi2"};

ACTS_DEBUG("invoke fitter");
auto result = fitTrack(trackSourceLinks, initialParams, chi2Options,
surfSequence, mtj);
surfSequence, *tracks);

if (result.ok()) {
ACTS_DEBUG("chi2algo | result ok");
ACTS_DEBUG("result ok");
// Get the fit output object
auto& fitOutput = result.value();
auto& track = result.value();
// The track entry indices container. One element here.
std::vector<Acts::MultiTrajectoryTraits::IndexType> trackTips;
trackTips.reserve(1);
trackTips.emplace_back(fitOutput.lastMeasurementIndex);
trackTips.emplace_back(track.tipIndex());
// The fitted parameters container. One element (at most) here.
Trajectories::IndexedParameters indexedParams;
ACTS_VERBOSE("chi2algo | final χ² = " << fitOutput.chisquare);
ACTS_VERBOSE("chi2algo | lastMeasurementIndex = "
<< fitOutput.lastMeasurementIndex);

if (fitOutput.fittedParameters) {
const auto& params = fitOutput.fittedParameters.value();
ACTS_VERBOSE("chi2algo | Fitted parameters for track "
<< itrack << ": " << params.parameters().transpose());
ACTS_VERBOSE("final χ² = " << chisquare(track));
ACTS_VERBOSE("lastMeasurementIndex = " << track.tipIndex());

if (track.hasReferenceSurface()) {
ACTS_VERBOSE("Fitted parameters for track "
<< itrack << ": " << track.parameters().transpose());
// Push the fitted parameters to the container
indexedParams.emplace(fitOutput.lastMeasurementIndex, params);
indexedParams.emplace(
std::pair{track.tipIndex(),
TrackParameters{track.referenceSurface().getSharedPtr(),
track.parameters(), track.covariance()}});
} else {
ACTS_DEBUG("chi2algo | No fitted parameters for track " << itrack);
ACTS_DEBUG("No fitted parameters for track " << itrack);
}
// store the result
trajectories.emplace_back(fitOutput.fittedStates, std::move(trackTips),
trajectories.emplace_back(trackStateContainer, std::move(trackTips),
std::move(indexedParams));
} else {
ACTS_WARNING("Fit failed for track "
Expand Down
5 changes: 3 additions & 2 deletions Examples/Algorithms/TrackFittingChi2/src/TrackFittingChi2AlgorithmFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ struct TrackFitterChi2FunctionImpl
const ActsExamples::TrackParameters& initialParameters,
const ActsExamples::TrackFittingChi2Algorithm::TrackFitterChi2Options&
options,
std::shared_ptr<Acts::VectorMultiTrajectory>& trajectory) const override {
ActsExamples::TrackFittingChi2Algorithm::TrackContainer& trackContainer)
const override {
return trackFitterChi2.fit(sourceLinks.begin(), sourceLinks.end(),
initialParameters, options, trajectory);
initialParameters, options, trackContainer);
};
};

Expand Down
10 changes: 7 additions & 3 deletions Tests/UnitTests/Core/TrackFitting/Chi2FitterTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "Acts/EventData/Measurement.hpp"
#include "Acts/EventData/SourceLink.hpp"
#include "Acts/EventData/TrackParameters.hpp"
#include "Acts/EventData/VectorTrackContainer.hpp"
#include "Acts/Geometry/GeometryContext.hpp"
#include "Acts/Geometry/GeometryIdentifier.hpp"
#include "Acts/Geometry/TrackingGeometry.hpp"
Expand Down Expand Up @@ -198,12 +199,15 @@ BOOST_AUTO_TEST_CASE(ZeroFieldNoSurfaceForward) {

// BOOST_TEST_INFO("Test Case ZeroFieldNoSurfaceForward: running .fit()...");

auto res =
chi2Zero.fit(sourceLinks.begin(), sourceLinks.end(), start, chi2Options);
Acts::TrackContainer tracks{Acts::VectorTrackContainer{},
Acts::VectorMultiTrajectory{}};

auto res = chi2Zero.fit(sourceLinks.begin(), sourceLinks.end(), start,
chi2Options, tracks);
BOOST_REQUIRE(res.ok());

const auto& val = res.value();
BOOST_CHECK(val.finished);
BOOST_CHECK(val.hasReferenceSurface());
}

// TODO: add more test cases, for holes, outliers, ...
Expand Down

0 comments on commit 13f15e1

Please sign in to comment.