-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Refactor the measurement calibrator in the examples #2058
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
3b25778
Refactor the measurement calibrator in the examples
gagnonlg 3cb08ea
Run "black"
gagnonlg 1ee31f0
Fix alignement example
gagnonlg e8c267a
Add missing include
gagnonlg 27304e1
Implement clang-tidy suggestions
gagnonlg 50d8ad2
Attempt to fix ubuntu build
gagnonlg ea65cbf
Avoid nullptr to calibrator
gagnonlg b176d7a
clang-tidy strikes again
gagnonlg 48d673f
clang-format
gagnonlg 236d6e5
Move Calibrators to EventData
gagnonlg ee06118
Rename PairedMeasurementCalibrator to MeasurementCalibratorAdapter
gagnonlg 052ab73
clang format
gagnonlg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
47 changes: 3 additions & 44 deletions
47
Examples/Framework/include/ActsExamples/EventData/Measurement.hpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,68 +1,27 @@ | ||
// This file is part of the Acts project. | ||
// | ||
// Copyright (C) 2020 CERN for the benefit of the Acts project | ||
// Copyright (C) 2020-2023 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/. | ||
|
||
#pragma once | ||
|
||
#include "Acts/EventData/Measurement.hpp" | ||
#include "Acts/EventData/MultiTrajectory.hpp" | ||
#include "Acts/EventData/SourceLink.hpp" | ||
#include "Acts/EventData/VectorMultiTrajectory.hpp" | ||
#include "ActsExamples/EventData/IndexSourceLink.hpp" | ||
#include <Acts/EventData/Measurement.hpp> | ||
|
||
#include <cassert> | ||
#include <vector> | ||
|
||
namespace ActsExamples { | ||
|
||
/// Variable measurement type that can contain all possible combinations. | ||
using Measurement = ::Acts::BoundVariantMeasurement; | ||
|
||
/// Container of measurements. | ||
/// | ||
/// In contrast to the source links, the measurements themself must not be | ||
/// orderable. The source links stored in the measurements are treated | ||
/// as opaque here and no ordering is enforced on the stored measurements. | ||
using MeasurementContainer = std::vector<Measurement>; | ||
|
||
/// Calibrator to convert an index source link to a measurement. | ||
class MeasurementCalibrator { | ||
public: | ||
/// Construct an invalid calibrator. Required to allow copying. | ||
MeasurementCalibrator() = default; | ||
/// Construct using a user-provided container to chose measurements from. | ||
MeasurementCalibrator(const MeasurementContainer& measurements) | ||
: m_measurements(&measurements) {} | ||
|
||
/// Find the measurement corresponding to the source link. | ||
/// | ||
/// @tparam parameters_t Track parameters type | ||
/// @param gctx The geometry context (unused) | ||
/// @param trackState The track state to calibrate | ||
void calibrate( | ||
const Acts::GeometryContext& /*gctx*/, | ||
Acts::MultiTrajectory<Acts::VectorMultiTrajectory>::TrackStateProxy | ||
trackState) const { | ||
const IndexSourceLink& sourceLink = | ||
trackState.getUncalibratedSourceLink().get<IndexSourceLink>(); | ||
assert(m_measurements and | ||
"Undefined measurement container in DigitizedCalibrator"); | ||
assert((sourceLink.index() < m_measurements->size()) and | ||
"Source link index is outside the container bounds"); | ||
std::visit( | ||
[&](const auto& meas) { | ||
trackState.allocateCalibrated(meas.size()); | ||
trackState.setCalibrated(meas); | ||
}, | ||
(*m_measurements)[sourceLink.index()]); | ||
} | ||
|
||
private: | ||
// use pointer so the calibrator is copyable and default constructible. | ||
const MeasurementContainer* m_measurements = nullptr; | ||
}; | ||
|
||
} // namespace ActsExamples |
65 changes: 65 additions & 0 deletions
65
Examples/Framework/include/ActsExamples/EventData/MeasurementCalibration.hpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
// This file is part of the Acts project. | ||
// | ||
// Copyright (C) 2023 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/. | ||
|
||
#pragma once | ||
|
||
#include "Acts/EventData/MultiTrajectory.hpp" | ||
#include "Acts/EventData/SourceLink.hpp" | ||
#include "Acts/EventData/VectorMultiTrajectory.hpp" | ||
#include "ActsExamples/EventData/IndexSourceLink.hpp" | ||
#include <ActsExamples/EventData/Measurement.hpp> | ||
|
||
#include <cassert> | ||
|
||
namespace ActsExamples { | ||
|
||
/// Abstract base class for measurement-based calibration | ||
class MeasurementCalibrator { | ||
public: | ||
virtual void calibrate( | ||
const MeasurementContainer& measurements, | ||
const Acts::GeometryContext& gctx, | ||
Acts::MultiTrajectory<Acts::VectorMultiTrajectory>::TrackStateProxy& | ||
trackState) const = 0; | ||
|
||
virtual ~MeasurementCalibrator() = default; | ||
}; | ||
|
||
// Calibrator to convert an index source link to a measurement as-is | ||
class PassThroughCalibrator : public MeasurementCalibrator { | ||
public: | ||
/// Find the measurement corresponding to the source link. | ||
/// | ||
/// @tparam parameters_t Track parameters type | ||
/// @param gctx The geometry context (unused) | ||
/// @param trackState The track state to calibrate | ||
void calibrate( | ||
const MeasurementContainer& measurements, | ||
const Acts::GeometryContext& /*gctx*/, | ||
Acts::MultiTrajectory<Acts::VectorMultiTrajectory>::TrackStateProxy& | ||
trackState) const override; | ||
}; | ||
|
||
// Adapter class that wraps a MeasurementCalibrator to conform to the | ||
// core ACTS calibration interface | ||
class MeasurementCalibratorAdapter { | ||
public: | ||
MeasurementCalibratorAdapter(const MeasurementCalibrator& calibrator, | ||
const MeasurementContainer& measurements); | ||
|
||
void calibrate( | ||
const Acts::GeometryContext& gctx, | ||
Acts::MultiTrajectory<Acts::VectorMultiTrajectory>::TrackStateProxy | ||
trackState) const; | ||
|
||
private: | ||
const MeasurementCalibrator& m_calibrator; | ||
const MeasurementContainer& m_measurements; | ||
}; | ||
|
||
} // namespace ActsExamples |
39 changes: 39 additions & 0 deletions
39
Examples/Framework/src/EventData/MeasurementCalibration.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// This file is part of the Acts project. | ||
// | ||
// Copyright (C) 2023 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 <ActsExamples/EventData/MeasurementCalibration.hpp> | ||
|
||
void ActsExamples::PassThroughCalibrator::calibrate( | ||
const MeasurementContainer& measurements, | ||
const Acts::GeometryContext& /*gctx*/, | ||
Acts::MultiTrajectory<Acts::VectorMultiTrajectory>::TrackStateProxy& | ||
trackState) const { | ||
const IndexSourceLink& sourceLink = | ||
trackState.getUncalibratedSourceLink().get<IndexSourceLink>(); | ||
assert((sourceLink.index() < measurements.size()) and | ||
"Source link index is outside the container bounds"); | ||
|
||
std::visit( | ||
[&trackState](const auto& meas) { | ||
trackState.allocateCalibrated(meas.size()); | ||
trackState.setCalibrated(meas); | ||
}, | ||
(measurements)[sourceLink.index()]); | ||
} | ||
|
||
ActsExamples::MeasurementCalibratorAdapter::MeasurementCalibratorAdapter( | ||
const MeasurementCalibrator& calibrator, | ||
const MeasurementContainer& measurements) | ||
: m_calibrator{calibrator}, m_measurements{measurements} {} | ||
|
||
void ActsExamples::MeasurementCalibratorAdapter::calibrate( | ||
const Acts::GeometryContext& gctx, | ||
Acts::MultiTrajectory<Acts::VectorMultiTrajectory>::TrackStateProxy | ||
trackState) const { | ||
return m_calibrator.calibrate(m_measurements, gctx, trackState); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is merged already, but I there a reason not to just expose the inheritance relationship and then have Python just construct the object directly instead of the factory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No real reason I guess, I just don't know what I'm doing on the python side ;-) putting this on my list