Proposal: CRTP track state creator#5264
Conversation
github-actions
bot
added
Component - Core
| const MeasurementClassification classification = | ||
| derived().classifyMeasurement(surface, measurement, chi2, | ||
| maxChi2Compatible, maxChi2Outlier); | ||
| result.emplace_back(i, chi2, classification); |
There was a problem hiding this comment.
What about only keeping classifications below the outlier cut ?
Not clear whether it is really an advantage, but if the results are added sorted, than the list could be limited to the maximum number of branches.
There was a problem hiding this comment.
I thought it might be convenient to have the outliers also in the list for downstream handling. but depends if this generates some overhead and if it really turns out to be useful
right, we can also insertion sort them. I guess this also depends on the performance
|
|
||
| std::optional<TrackStateProxy> firstTrackState; | ||
|
|
||
| if (derived().hasHole(surface, boundState, selectedMeasurements, logger)) { |
There was a problem hiding this comment.
does it make sense to add holes and measurements at the same time?
There was a problem hiding this comment.
not sure, but I can imagine that scenarios where it is not clear if you should pick a measurement or produce an (edge) hole. ultimately it should be up to the user to choose so I allowed for this for now
| ACTS_VERBOSE("Found " << measurementRange.size() | ||
| << " measurements on surface " | ||
| << surface.geometryId()); | ||
| if (measurementRange.begin() == measurementRange.end()) { |
There was a problem hiding this comment.
what about support for "edge holes" ?
The derived class could test in case of no-measurements (same applies for the case that there are no measurements which pass the outlier cut), whether the prediction is close to the edge with a negative boundary tolerance and return the special error code for no measurement expected.
There was a problem hiding this comment.
good point - I wasn't sure how this is handled right now
I think it would be more convenient if the track state creator would also deal with holes / edge holes so we can pull this logic out of the CKF. but one step at a time. I will make this compatible with the current version
| const Surface& surface, const BoundState& boundState, | ||
| const TrackIndexType prevTip, | ||
| std::vector<track_state_proxy_t>& trackStateCandidates, | ||
| track_state_container_backend_t& trajectory, const Logger& logger) const { |
There was a problem hiding this comment.
Logger gets passed to almost every function here: at that point, maybe this should be a class member?
There was a problem hiding this comment.
Currently the logger is part of the track state creator interface so it might be confusing to add a second one
|
3 participants
Proposal for a CRTP track state creator which provides helpers and customization points to simplify and generalize the implementation of a track state creator.
We saw quite some performance improvements in Athena after the implementation of a track state creator as it cuts the overhead of passing all the measurements through the track state EDM. Making this available in ACTS should equally improve the performance in our Examples framework and allow users to move away from the old source link accessor solution more easily.
This implementation should also make it possible to split measurements into multiple track states which is necessary for the Run 4 track finding in dense environment prototype.
This further generalizes the track state creation to holes and outliers which ultimately gives to user more control over the track finding procedure. For example, one can easily reject a hole and continue the finding if the track just clipped a sensor. Or one might branch into a hole + measurement / outlier + measurement / edge hole + measurement state due to ambiguity.
Further ideas
--- END COMMIT MESSAGE ---