RPC DQM occupancy bugfix + cleanup #20356
Conversation
|
The code-checks are being triggered in jenkins. |
|
A new Pull Request was created by @jhgoh (Junghwan John Goh) for master. It involves the following packages: DQM/RPCMonitorDigi @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20356/462 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-20356/462/git-diff.patch You can run |
|
The code-checks are being triggered in jenkins. |
|
Pull request #20356 was updated. @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please check and sign again. |
|
+code-checks |
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
@davidlange6 I find that suggestion was a just temporary "workaround". The issue back then was that alphanumeric histograms could be implicitly extended via fill / findBin. (axis extension is problematic for DQM, since we can no longer use TH1::Add()). And it was fixed by this commit, root-project/root@1796bf2 This RPC DQM bug looks very similar to how an axis extension would look like. Either way, this PR should be merged. If a subsystem/module wants to explicitly avoid having alphanumeric histograms, they should disable it just like it was done in this PR. |
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
The tests are being triggered in jenkins. |
|
This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1 |
|
@dmitrijus should I prepare backport(s) to apply this bugfix as well? |
|
@jhgoh That would welcome, yes :) |
|
93X backport is created. Better to prepare one more for 92X? |
This PR fixes problems in the RPC offline DQM.
Problem:
In the RPC summary plots (offline DQM), ring3/A,C shows very low occupancy in 2D plots (https://goo.gl/sWUTxU). This is not consistent with <Run2016 nor the online DQM. It also disagree with 1D distributions. Same symptoms are observed in release validation.
Observation:
We could reproduce the same problem with RelVal samples with multi-threading.
The distribution becomes OK with single thread mode.
Affected histograms have custom labels in each axis, without calling setBinLabel() restores nominal shapes.
Solution:
Explicitly call TAxis::SetNoAlphanumeric() for the MonitorElements before setting bin labels.
Other changes:
We also added some changes for cleanup the codes and remove obsolete scripts.
@slezki @mileva @andresib