-
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
Tracking DQM: miscellaneous fixes #28823
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28823/13543
|
A new Pull Request was created by @mmusich (Marco Musich) for master. It involves the following packages: DQM/TrackingMonitor @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
+1 |
Comparison job queued. |
Hi @mmusich , while you are at it, please be aware of this observation: #28622 (comment) Specifically:
This is not really a big problem since all analyzers fill the same plots with the same values, but after #28622 these plots will have an inflated number of entries (since all the fill's will go to the same histogram). Currently, the different module instances fill independent histograms and an arbitrary one of these will be saved in the end, which does not make much sense but is fine as long as all the modules produce the same output. |
@schneiml thanks for the heads-up but this goes well beyond the scope of this PR. Let's keep it separate. Thanks. |
Comparison is ready Comparison Summary:
|
@mmusich Apart from the changes you described in the PR header, it looks like there are some plots which get no entries now in Tracking / TrackParameters / TrackEfficiency , those with E/eff string in the name, I guess computing efficiencies |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28823/13558
|
Pull request #28823 was updated. @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please check and sign again. |
please test |
The tests are being triggered in jenkins. |
+1 |
Comparison job queued. |
@mtosi to be more specific, e.g.
I assume it's the same for all in this folder. IIRC there are more colliding groups in other workflows. |
Comparison is ready Comparison Summary:
|
+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. @davidlange6, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
The aim of this PR is fix various plots in Tracking DQM:
DistanceOfClosestApproachToBSdz
plot originally included in New plots for Tracking Workspace #27330 has x-axis range too small (actually ~ half of the statistics goes into the overflow bins right now, see e.g. https://tinyurl.com/smcjqap)DQM/TrackingMonitorSource/python/TrackingSourceConfigTier0_Cosmic_cff.py
is apparently an unused copy ofDQM/TrackingMonitorSource/python/TrackingSourceConfig_Tier0_Cosmic_cff.py
, so it is removed.PR validation:
Minimal standard integration tests have been run:
runTheMatrix.py -l 11634.0 -t 4 -j 8 --command='-n 100'
runTheMatrix.py -l 7.22,7.21,7.4,7.3,138.1 -t 4 -j 8
The Beam Spot distance of closest approach with the new x-axis looks like this:
if this PR is a backport please specify the original PR and why you need to backport that PR:
This is not a backport and no backport is intended.
cc:
@mtosi @sroychow @arossi83