-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[12.5.X] fix ClusterStorer::ClusterHitRecord<T>::rekey
for Phase2TrackerRecHit1D
#40559
[12.5.X] fix ClusterStorer::ClusterHitRecord<T>::rekey
for Phase2TrackerRecHit1D
#40559
Conversation
A new Pull Request was created by @mmusich (Marco Musich) for CMSSW_12_5_X. It involves the following packages:
@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
type bug-fix, tracking |
backport of #40548 |
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1be805/30071/summary.html Comparison SummarySummary:
|
+reconstruction |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_5_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_13_0_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
This is a follow-up to #40395.
In the specialization of
ClusterStorer::ClusterHitRecord<T>::rekey
forVectorHit
s introduced there, I overlooked thatVectorHit::ClusterRef
andPhase2TrackerRecHit1D::ClusterRef
aretypedefs
of the exact same object:cmssw/DataFormats/TrackerRecHit2D/interface/Phase2TrackerRecHit1D.h
Line 13 in 8154525
cmssw/DataFormats/TrackerRecHit2D/interface/VectorHit.h
Line 30 in 8154525
This leads to the fact that when this function was called here:
cmssw/CommonTools/RecoAlgos/src/ClusterStorer.cc
Line 133 in 7661a79
with
Phase2TrackerRecHit1D::ClusterRef
specialization, it didn't capture any cluster reference but was just returning:cmssw/CommonTools/RecoAlgos/src/ClusterStorer.cc
Lines 198 to 200 in 7661a79
This leads to broken cluster reference for
Phase2TrackerRecHit1D
in Phase-2 workflows.PR validation:
Run successfully:
and tested that I can correctly refit the resulting tracks with the machinery introduced at #40542.
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
Verbatim backport of #40548. Needed to backport all the way down to 12.5.X for phase-2 related samples productions.