-
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
Fix Tracks related Pixel quantities at HLT DQM #40873
Conversation
resolves #30911 |
resolves CMSTrackerDPG/Tasks#5 |
test parameters:
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40873/34341
|
A new Pull Request was created by @arossi83 (Alessandro Rossi) for master. It involves the following packages:
@emanueleusai, @cmsbuild, @syuvivida, @rvenditti, @micsucmed, @pmandrik can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
#+ hltSiPixelPhase1TrackClustersAnalyzer | ||
hltSiPixelClusterShapeCache | ||
+ hltSiPixelPhase1ClustersAnalyzer | ||
+ refittedForPixelDQM |
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.
shouldn't this be
+ refittedForPixelDQM | |
+ hltrefittedForPixelDQM |
?
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.
Yes, It should. Just push a commit to fix it.
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.
Thanks, can you apply the rest of the suggestions at #40875 (comment) ? In that way we should be passing tests without need to change any input.
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.
Done, sorry I overlooked the comments yesterday
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40873/34342
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40873/34355
|
Pull request #40873 was updated. @micsucmed, @emanueleusai, @clacaputo, @cmsbuild, @syuvivida, @pmandrik, @mandrenguyen, @rvenditti can you please check and sign again. |
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-573eb9/30902/summary.html Comparison SummarySummary:
|
Hello let's see if we can track down all the changes in the DQM comparisons:
|
Hi @emanueleusai,
This is needed to be able to monitor the performance of the pixel detector - as seen by the HLT - during 2024, as we we expect it to have to cope with unprecedentedly hard conditions from the machine (well beyond the specs of the readout chip at the peak luminosity during the luminosity leveling phase of the fill). This will help to track down possible sources of tracking inefficiency at the HLT.
Yes, this in fact corrects a "bug" in the offline pixel DQM. Since when the trajectory is a transient event product, in order to compute precise residuals a refit of the track is needed. If that is not provided the DQM module falls back to use the pre-computed track residuals, stored in the
This indeed looks unrelated, and instead potentially related to the known instability of wf 11634.7, as reported already in a separate issue: #39803 |
Thank you very much for the detailed follow-up. Everything is understood and as expected from the code changes. |
+1 |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). 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) |
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.
Given the urgency of the backport, to be ready in time for 13_0_0, please apply this fix in a forthcoming PR while we are merging this one. Possibly now, so that it can be also merged in the backport
|
||
hltSiPixelClusterShapeCache = siPixelClusterShapeCache.clone(src = 'hltSiPixelClusters') | ||
hltrefittedForPixelDQM = refittedForPixelDQM.clone(src ='hltMergedTracks', | ||
TTRHBuilder = cms.string('WithTrackAngle')) # no templates at HLT |
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.
TTRHBuilder = cms.string('WithTrackAngle')) # no templates at HLT | |
TTRHBuilder = 'WithTrackAngle') # no templates at HLT |
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.
+1 |
PR description:
Monitor of the tracks related Pixel quantities at HLT DQM never works and it was disabled at some point, with this PR we introduce the changes needed (tracks refitting) in order to be able to monitor Pixel Tracks quantities at HLT DQM.
In this PR we also update the collection to be used by the Standard DQM Pixel Residuals Monitor in order to use the refitted tracks.
PR validation:
runTheMatrix.sh -l 140.063
Manually run the Online HLT client at P5 with old stored DQM stream file
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:
Backport to 13_0_X will be needed since it's the release which will be used for 2023 datataking