feat: Demonstrator for MillePede alignment of ACTS Kalman tracks#5256
feat: Demonstrator for MillePede alignment of ACTS Kalman tracks#5256goblirsc wants to merge 16 commits intoacts-project:mainfrom
Conversation
goblirsc
changed the title
|
Wow, the CI even checks the PR title? |
github-actions
bot
added
Component - Examples
|
Amazing! |
|
Pausing updates for now to allow for review (sorry for the big PR - once the first bits were in place the extra features just kept coming by themselves 😇 ) |
SonarQube is an &*$% minor cosmetics
goblirsc
force-pushed
the
MG_MillePede_demo
branch
from
ff4d2f2 to
38176e7
Compare
|
Could someone please review this? |
|
It is also on my list but I didn't get to it yet. Let me also ping the review channel on MM https://mattermost.web.cern.ch/acts/channels/code-review |
andiwand
left a comment
andiwand
left a comment
There was a problem hiding this comment.
couple of small comments and nitpicks but didn't go through completely yet
one major change I was not sure about yet is the telescope detector modifications. I thought the alignment of our detectors is already in place and general enough to support this. I think @asalzburger knows more about this
There was a problem hiding this comment.
I wonder if we could get rid of Acts in the file and class name. how about MillePedeAlignmentAlgorithm?
There was a problem hiding this comment.
Would prefer to keep - as this algorithm is not running Millepede but instead the ACTS-Internal alignment solver, hence the ActsSolver prefix. A MillePedeAlignmentSolver calling pede (the Millepede solver program) can / will be added later :)
...hms/AlignmentMillePede/include/ActsExamples/AlignmentMillePede/MillePedeAlignmentSandbox.hpp
Show resolved
Hide resolved
...lgorithms/AlignmentMillePede/include/ActsExamples/AlignmentMillePede/ActsSolverFromMille.hpp
Outdated
Show resolved
Hide resolved
| ProcessCode ActsSolverFromMille::execute( | ||
| const AlgorithmContext& /*ClangShutUp*/) const { | ||
| // this algorithm will not do anything at event-time | ||
| return ProcessCode::SUCCESS; | ||
| } |
There was a problem hiding this comment.
if the algorithm does not do anything during execution should this rather be a "tool"?
There was a problem hiding this comment.
It does something during finalisation, and in terms of the workflow takes the role of an algorithm (= scheduled step of the processing pipeline), rather than being called from another algorithm. So would prefer to keep this an algorithm.
|
| /// add a tiny diagonal matrix for additional stabilisation | ||
| auto eigensolver = Eigen::SelfAdjointEigenSolver<Acts::DynamicMatrix>( | ||
| inputCov + | ||
| 1.e-10 * Acts::DynamicMatrix::Identity(inputCov.rows(), inputCov.cols())); |
There was a problem hiding this comment.
1e-10 stabilization parameter could be made configurable here?
| double lambdaMax = eigenVals(eigenVals.size() - 1); | ||
| // check for a huge leading eigenvalue - this happens when the time coordinate | ||
| // is unconstrained | ||
| if (removeLargeLeading && lambdaMax > 100 * eigenVals(maxIndex - 1)) { |
There was a problem hiding this comment.
Same comment on 1e2, perhaps configurable?
| unsigned int iSurface = 0; | ||
| for (auto& [geoID, surface] : sortedGeo) { | ||
| // only consider sensitive surfaces | ||
| if (geoID.sensitive() == 0) { |
There was a problem hiding this comment.
| if (geoID.sensitive() == 0) { | |
| if (!surface->isSensitive()) { |
since recently we have a shorthand for this on the surface. could be nice to decouple this from the geo id
| unsigned int iSurface = 0; | ||
| for (auto& [geoID, surface] : sortedGeo) { | ||
| // only consider sensitive surfaces | ||
| if (geoID.sensitive() == 0) { |
There was a problem hiding this comment.
| if (geoID.sensitive() == 0) { | |
| if (!surface->isSensitive()) { |
| << " +/- " << std::setw(10) | ||
| << std::sqrt(alignResult.alignmentCovariance(row, row)) | ||
| << std::endl; | ||
| ; |
| /// and stored e.g. in a DB file for further use / validation. | ||
| /// For this initial demo, we just print them out. | ||
|
|
||
| std::cout << "Performed internal alignment. " << std::endl; |
There was a problem hiding this comment.
I think these couts should be converted to use the logger
|
|
||
| return ProcessCode::SUCCESS; | ||
| } | ||
| ProcessCode MillePedeAlignmentSandbox::finalize() { |
There was a problem hiding this comment.
| ProcessCode MillePedeAlignmentSandbox::finalize() { | |
| ProcessCode MillePedeAlignmentSandbox::finalize() { |
| inline const std::vector<std::unique_ptr<Acts::Transform3>>& | ||
| TelescopeDetectorElement::alignedTransforms() const { | ||
| return m_alignedTransforms; | ||
| } | ||
|
|
||
| inline TelescopeDetectorElement::Identifier | ||
| TelescopeDetectorElement::identifier() const { | ||
| return m_elementIdentifier; | ||
| } |
There was a problem hiding this comment.
we could just move these one liners to the declaration
There was a problem hiding this comment.
ok I see - should we just make the TelescopeDetector alignable instead of adding another class?
| /// @param record: Mille record to write to - should be valid pointer | ||
| /// Note: Not very efficient - we have to "un-fit" the kalman track. | ||
| /// Used for R&D, recommending the GBL track model (under development) | ||
| // for production use. |
There was a problem hiding this comment.
| // for production use. | |
| /// for production use. |
| /// Used for R&D, recommending the GBL track model (under development) | ||
| // for production use. | ||
| void dumpToMille(const ActsAlignment::detail::TrackAlignmentState& state, | ||
| MilleRecord* record); |
There was a problem hiding this comment.
in case record should not be nullptr we could express this by using a reference MilleRecord& record
5 participants
Add a first demonstrator of MillePede alignment, using existing ACTS Kalman tracks.
--- END COMMIT MESSAGE ---
First step towards adding actual life to the MillePede ❤️ ACTS effort!
After making
Milleavailable as a plugin in we use it to write ACTS-Kalman tracks out into binary files that can be fit with the MillePede package.This path is simple (as we can re-use existing tracks), but computationally highly inefficient - we have to "undo" the track fit to bring the global covariance matrix of the Kalman tracks into the format expected by Millepede.
This should hence be seen as an initial proof of concept demonstrator.
For real-life usage, the
GeneralBrokenLinestrack refit (planned for ACTS-integration in the near future) will likely be a better choice for the fitter component in the alignment. But the final implementation should be sufficiently modular to work with any fitter nonetheless.In this MR, we add
Millebinaries, allowing to populate ACTSTrackAlignmentStates from Mille binariesAlignedTelescopeDetectorfor a simple showcase setupMillewriting and reading back into ACTS results in consistent alignment problemsThe alignment fit itself is performed outside ACTS using the MillePede package. Alternatively, it is possible to read the binaries back and solve using the ACTS tooling.