-
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
PPS Pixel DQM: assign errors to right plots #41649
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41649/35528
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41649/35529
|
A new Pull Request was created by @fabferro (Fabrizio Ferro) for master. It involves the following packages:
@nothingface0, @emanueleusai, @cmsbuild, @pmandrik, @syuvivida, @tjavaid, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
-1 Failed Tests: RelVals RelVals
|
It looks like auto:run2_mc GT (ie: 123X_mcRun2_asymptotic_v1) does not contain CTPPSPixelDAQMappingRcd. It is needed in the updates introduced by this PR. It should be added. @wpcarvalho |
type ctpps |
Hi @fabferro the So something must be wrong in the PR if you are trying to consume that record in workflows older than 2018...maybe you need to restrict this to Run3 workflows only? |
@francescobrivio : I did not remember that discussion. You are suggesting to use era_modifiers? |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41649/35574
|
Pull request #41649 was updated. @nothingface0, @emanueleusai, @cmsbuild, @pmandrik, @syuvivida, @tjavaid, @micsucmed, @rvenditti can you please check and sign again. |
@cmsbuild , please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0814e4/32677/summary.html Comparison SummaryThere are some workflows for which there are errors in the baseline: 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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
@fabferro I am puzzled: in #41649 (comment) is written that "the CTPPSPixelDAQMappingRcd were introduced in the MC GTs only for 2018 and Run3". Ok for clearing the ctppsDQMOfflineSource from runs in the |
Actually the campaign for updating PPS MC tags started from 2017, at least for pixels. I'm not 100% sure that ALL MC GTs were updated, but all used 131X_mc2017_* were. |
Ok, thank you @fabferro : this explains why we got no errors from the 2017 workflows |
+1 |
PR description:
This PR is a bug fix in filling the plots in PPS Pixel DQM. Using the DAQ map, errors coming from pixels are assigned to the right detector plane instead of being put in a unidentified-detector plot, unless strictly necessary.
PR validation:
With matrix wfs 138.4 (run3) and 136.874 (run2) and with brand new 2023 data. Errors are now put inside the right plots.
This fix is needed for the data taking asap. Backports are therefore needed.