Skip to content
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

[RPC] Changing the RPCDcsInfo* about an issue of the DQM legacy module #31056

Merged
merged 8 commits into from Aug 18, 2020

Conversation

jeongsumin
Copy link
Contributor

@jeongsumin jeongsumin commented Aug 5, 2020

PR description:

The RPCDcsInfo module has a problem because of using DQMOneLumiEDAnalyzer.
(This problem was already issued in the #25090 )

This module gets the RPC DCS bit per Lumi-Section using 'scalersRawToDigi'.
So in the harvesting step, RPCDcsInfoClient also affected by this problem.
But DCS bit already contained in the reportSummaryMap which is histogramming at the DQMProvInfo module.
Therefore, we do not need to use the RPCDcsInfo module and the luminosity block (dqmEndLuminosityBlock) in the RPCDcsInfoClient.

From this change, RPC does not have an issue with this.

At the bottom, I attach the RPC HV status plot which affected by RPC DCS bit.
before & after changing the module:

  • Fixing the bug: the HV status bit has filled at the "lumi_number+1"th bin. So I change that.
    ** HV status histogram: If the scalersRawToDigi (DCS bit) not found, then HV status is 0 (HV was off). If not, then HV status is 1 (HV was on)
    *** This data has 576 LumiSection

changing module - before and after

PR validation:

Tested by runTheMatrix 138.1 (RunCosmics2020, MWGR data) with multi-thread and single-thread

if this PR is a backport please specify the original PR and why you need to backport that PR:

@andresib @jhgoh
I make the new PR about changing the RPC DQM legacy module.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2020

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31056/17567

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31056/17593

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31056/17594

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

A new Pull Request was created by @jeongsumin (jeongsumin) for master.

It involves the following packages:

DQM/RPCMonitorClient
DQM/RPCMonitorDigi

@kmaeshima, @cmsbuild, @andrius-k, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@acimmino this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@@ -7,5 +7,3 @@ DEFINE_FWK_MODULE(RPCMonitorDigi);
DEFINE_FWK_MODULE(RPCRecHitProbability);
#include "DQM/RPCMonitorDigi/interface/RPCTTUMonitor.h"
DEFINE_FWK_MODULE(RPCTTUMonitor);
#include "DQM/RPCMonitorDigi/interface/RPCDcsInfo.h"
DEFINE_FWK_MODULE(RPCDcsInfo);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are making it so the module can never be loaded, then shouldn't RPCDcsInfo.h and RPCDcsInfo.cc both be removed from CMSSW?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to update the removal... Thank you for mentioning that!

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3cf0ea/8789/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2608220
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2608197
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.852 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 1000.0,... ): -0.042 KiB RPC/DCSInfo
  • DQMHistoSizes: changed ( 10024.0,... ): -0.030 KiB RPC/DCSInfo
  • Checked 149 log files, 22 edm output root files, 35 DQM output files

@jfernan2
Copy link
Contributor

+1
Thanks @jeongsumin

@cmsbuild
Copy link
Contributor

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Aug 18, 2020

+1

@mrodozov
Copy link
Contributor

After this PR was merged, this unit test started failing in all IBs
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_11_2_X_2020-08-18-2300/unitTestLogs/DQM/Integration#/
with:

9-Aug-2020 03:52:23 CEST  Successfully opened file root://eoscms.cern.ch//eos/cms/store/user/cmsbuild/store/express/Commissioning2019/ExpressCosmics/FEVT/Express-v1/000/334/393/00000/D0F052ED-9CA5-F547-BA73-2AA370D51AE8.root
19-Aug-2020 03:52:32 CEST  Closed file root://eoscms.cern.ch//eos/cms/store/user/cmsbuild/store/express/Commissioning2019/ExpressCosmics/FEVT/Express-v1/000/334/393/00000/D0F052ED-9CA5-F547-BA73-2AA370D51AE8.root
----- Begin Fatal Exception 19-Aug-2020 03:52:32 CEST-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'RPCDcsInfo' in category 'CMS EDM Framework Module'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------

@jfernan2
Copy link
Contributor

@jeongsumin I believe it has to do with the Online client in
https://github.com/cms-sw/cmssw/blob/master/DQM/Integration/python/clients/rpc_dqm_sourceclient-live_cfg.py#L159
after the removal of the plugin:
#31056 (comment)
Can you please have a look ASAP?

@jeongsumin
Copy link
Contributor Author

@jfernan2 Thanks for checking that. I will see that right now.

@jeongsumin
Copy link
Contributor Author

@jfernan2 @mrodozov Sorry for the late reply.
When I checked the rpc_dqm_sourceclient-live_cfg.py that remove the rpcDcsInfo at #L159 and https://github.com/cms-sw/cmssw/blob/master/DQM/Integration/python/clients/rpc_dqm_sourceclient-live_cfg.py#L105,
there was no fatal exception about RPCDcsInfo.
But I saw the error message from RPCMonitorDigi that might be occurred when the muon input tag is not valid.
https://github.com/cms-sw/cmssw/blob/master/DQM/RPCMonitorDigi/src/RPCMonitorDigi.cc#L182

Have you ever seen that message when running the unit test? I don't know the reason why this error can be occurred for now.

@jfernan2
Copy link
Contributor

@jeongsumin you removed the plugin in:
https://github.com/cms-sw/cmssw/pull/31056/files#diff-1fd1af75a5154bfd453f1cd93273aad4L10
hence Online DQM client cannot find the plugin: the crash happens at runing even if the cfi defines it

@jeongsumin
Copy link
Contributor Author

@jfernan2 I forgot to do the unit test by using the before merged version yesterday. I'm so sorry about that.
(before merged version: RPCDcsInfo module exist in the RPCMonitorDigi)

The "[RPCMonitorDigi]: Muons - Product not valid for event" error also occurs at the before merged version when doing the unit test using by RPC online client. I'm not sure, but It seems to some option that the muon information can be used when the unit test.
@mrodozov Is there any special option about using the muon tag when running the RPC online client? I would very appreciate your answer.

So I think if the RPCDcsInfo process is removed from RPC online client, there will be no more test failing in all IBs.
And If it is no problem to open the new PR about the modification of RPC online client, then I would make that after your answer.

@jfernan2
Copy link
Contributor

@jeongsumin if RPC is fine removing the module from Online DQM, we are fine too :-)

@jeongsumin
Copy link
Contributor Author

@jfernan2 Thanks a lot. Then I will make the PR about this right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants