-
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 for the cosmetic issue for the GEM DQM summary plot #45531
Fix for the cosmetic issue for the GEM DQM summary plot #45531
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45531/41002 |
A new Pull Request was created by @quark2 for master. It involves the following packages:
@antoniovagnerini, @cmsbuild, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
+1 Size: This PR adds an extra 28KB to repository
Comparison SummarySummary:
|
By the way, the cosmetic issue occurs on the run 382204, for example. If a replay is performed, is it possible to replay this run? |
please test
|
+1 Size: This PR adds an extra 16KB to repository Comparison SummarySummary:
|
I've checked that we have the desired differences in GEM DQM histograms. As you can see from the code, the differences are not displayed in the test GUI since they are on some underflow bins. I downloaded the histograms and checked them by myself. |
Hi @quark2 , thanks for checking. We are also going to test this PR online at our playback machines. Get back to you soon. |
hi @quark2 , all the clients are running without any crash while we test it on the top of |
Hi @tjavaid, Thanks for the test! The plots look good. However, the issue occurs only on the offlineDQM with a multicore environment. Although I have already tested this PR locally before, is it possible to test it in this environment? Best regards, |
+1 Size: This PR adds an extra 12KB to repository Comparison SummarySummary:
|
Hi @quark2 , We have tested this PR again on our playback machines. Looking at GUI plots, the summary plot looks okay. You can have a look and let us know. In addition, with bin-by-bin plots from jenkins tests, you can have a look at (e.g. for |
Hi @tjavaid, Sorry for my late... |
+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. @sextonkennedy, @mandrenguyen, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
Yep, sure. I'll prepare the backport. |
Milestone for this pull request has been moved to CMSSW_14_2_X. Please open a backport if it should also go in to CMSSW_14_1_X. |
@quark2 , could you please remove |
+1 |
type gem |
PR description:
In this PR, a cosmetic bug in the GEM DQM summary plots has been fixed. The bug occurs because we did not take into account the effect of the multi-core environment on the storage of the geometry information in histograms. The geometry information will be retrieved correctly by this fix.
PR validation:
Tests are done with
runTheMatrix.py -l 141.107
with an additional run (367142) andrunTheMatrix.py -l limited -i all --ibeos
.@jshlee @watson-ij @Dongwoon77