-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add a Phase2 HLT tracking DQM monitoring #42783
Add a Phase2 HLT tracking DQM monitoring #42783
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42783/36892
|
A new Pull Request was created by @mmusich (Marco Musich) for master. It involves the following packages:
@cmsbuild, @rappoccio, @nothingface0, @emanueleusai, @davidlange6, @pmandrik, @syuvivida, @antoniovilela, @tjavaid, @micsucmed, @fabiocos, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b0a0d2/34743/summary.html Comparison SummarySummary:
|
…entContent in the phase-2 case
…ing in the phase-2 case
…one in the case of the phase-2 HLT monitoring
3d5c18f
to
fa291dd
Compare
In the last push I add fa291dd which substitutes the Phase1 pixel cluster monitoring (useless for phase-2) and introduces the Phase-2 IT and OT cluster monitoring. Also I get rid of several other sequences that produce (currently) empty plots in the Phase-2 HLT. Respective POGs are welcome to add on to the sequence once they are ready to do so. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42783/36897
|
) | ||
|
||
# remove Strip HLT monitoring in the phase-2 sequence | ||
from Configuration.Eras.Modifier_phase2_tracker_cff import phase2_tracker |
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.
phase2_common
is not more appropriate here, to represent what it will do (i.e. for future when other modules will be added back) ?
However, it should give the same result as phase2_tracker
is always in phase-2 workflow.
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.
Well at the moment, it's fully tracker specific, so I'd rather not change it. In case it gets updated you can make sure the appropriate modifier is used
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.
OK, thx.
Just for the sake of due diligence: the PR adds about 400MB of histogram memory. Is it justified? |
As mentioned in #39362 this PR addresses (partially) the lack of monitoring of HLT objects. I would say it is justified as currently there is no proper way to check that PRs acting on the phase2 HLT menu are in line with expectations. Recent history shows that there were mistakes introduced (e.g. see #39323) that would be avoidable. So this PR is desireable from the HLT point of view.
Minimal in the sense that I didn't add all the possible HLT monitoring plots (that are by the way already included in Run3 relvals) for other objects, as I am not an expert in those. |
Thank you very much for the feedback |
+1 |
@cms-sw/orp-l2 your |
+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 be automatically merged. |
@mmusich |
no particular issues to speak of. The changes necessary are simply more involved to ensure the right association with Phase-2 tracker hits than just adding the DQM. I feel like TRK POG at HLT would be better suited to make these changes, if at the moment there is no manpower available I can also have a look. |
@vjmastra @lguzzi |
PR description:
Minimal set of changes needed to the Tracking @ HLT DQM monitoring sequence for the Phase-2 setup.
This relies on the current naming of the tracking collections for the phase-2 HLT:
generalTracks
(somewhat copied from offline)hltPhase2PixelTracks
(pixel tracks)hltPhase2PixelVertices
(pixel vertices)The latter two were introduced in #42491 but the name change was not percolated to the
FEVTDEBUGHLTEventContent
, so it becomes necessary to include them in the list of collections to be persisted. This is done in 8b2b22d.Changes for making the Phase-2 HLT Validation are more extensive and will be addressed with another PR at a later time.
PR validation:
Run successfully
runTheMatrix.py -l 24834.0 -t 4 -j 8 --ibeos
and checked that HLT tracking plots are filled.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:
N/A