refactor!: Add ability to copy Tracks between containers #1980
Conversation
Also allow to ensure matching dynamic columns
I didn't add this to the container itself because I'm not sure this being available is an assumption we should make (not sure though).
paulgessinger
force-pushed
the
fix/selector-perf
branch
from
d82aade
to
e364c81
Compare
|
fyi @tboldagh |
Codecov Report
@@ Coverage Diff @@
## main #1980 +/- ##
==========================================
+ Coverage 49.75% 49.80% +0.05%
==========================================
Files 413 413
Lines 23526 23596 +70
Branches 10652 10676 +24
==========================================
+ Hits 11705 11753 +48
- Misses 4338 4342 +4
- Partials 7483 7501 +18
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Looks good to me. I don't have much comment on this; I guess we can wait until tomorrow in case someone else wants to say something. If not, I can approve it tomorrow afternoon.
Concerning the selector, I guess the difference is that since you only copy the correct entry instead of removing the incorrect one, you don't have to resize after every deletion ?
📊 Physics performance monitoring for 6034147Full report VertexingVertexing vs. muIVF seededIVF truth_smearedIVF truth_estimatedIVF orthogonalSeedingSeeding seededSeeding truth_estimatedSeeding orthogonalCKFCKF seededCKF truth_smearedCKF truth_estimatedCKF orthogonalAmbiguity resolutionseededTruth tracking (Kalman Filter)Truth trackingTruth tracking (GSF)Truth tracking |
Examples/Algorithms/TruthTracking/ActsExamples/TruthTracking/TrackSelector.cpp
Show resolved
Hide resolved
Examples/Algorithms/TruthTracking/ActsExamples/TruthTracking/TrackSelector.cpp
Outdated
Show resolved
Hide resolved
paulgessinger
changed the title
Exactly. Also we only keep 1% of tracks for ttbar, so it's overall many fewer operations. |
|
@tboldagh @Corentin-Allaire @CarloVarni can we get this in? |
Yes from my side. |
Also yes from mine ! |
|
fine by me |
This preserves the dynamic columns if the backend supports this. Also adds a method to the
TrackContainerlayer that should ensure the backend copies the types using the correct types.This PR also switches the
TrackSelectoralgorithm to use this, instead of copying the input tracks and removing the discarded tracks. In my testing, this brings the selector performance back in line with expectations on ttbar.BREAKING CHANGE:
TrackContainerbackends needs to implement these methods:copyDynamicFrom_implensureDynamicColumns_impl