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

Add DMQ FakeBeamMonitor plugin and clients #30690

Merged
merged 3 commits into from
Jul 27, 2020

Conversation

francescobrivio
Copy link
Contributor

@francescobrivio francescobrivio commented Jul 14, 2020

PR description:

  • Added one new plugin under DQM/BeamMonitor: DQM/BeamMonitor/plugins/FakeBeamMonitor.cc

    • It's almost a copy of the BeamMonitor.cc plugin, except that the BeamSpot is not fitted from tracks and PVs, but is randomly generated using a TRandom3.
    • No change in the undelying logic with respect to the BeamMonitor is added
    • This plugin should be used when tracks/PVs are not available in the event, but a random BeamSpot is still needed (e.g. during MWGR)
  • Added two clients (basically a copy of the already existing beamspot clients) that run the new FakeBeamMonitor plugin:

    • DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py
    • DQM/Integration/python/clients/beamhltfake_dqm_sourceclient-live_cfg.py

PR validation:

The new plugin and clients have been tested "offline" by me, @gennai and @ggovi using and "EmpySource" and thanks to @smorovic who was running a DAQ with fake runs.
Two customizations are needed in the clients:

Backporting

A backport to CMSSW_11_1_X might be needed.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30690/16982

  • This PR adds an extra 40KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @francescobrivio for master.

It involves the following packages:

DQM/BeamMonitor
DQM/Integration

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

cms-bot commands are listed here

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 14, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+1
Tested at: dfaab53
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-03e003/7941/summary.html
CMSSW: CMSSW_11_2_X_2020-07-14-1100
SCRAM_ARCH: slc7_amd64_gcc820

@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-03e003/7941/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2787429
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2787378
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 154 log files, 17 edm output root files, 37 DQM output files

@andrius-k
Copy link

Hello,
Could you please provide a backport of this PR to 11_1_X for us (DQM) to test it in the playback system?
Regards,
Andrius

@francescobrivio
Copy link
Contributor Author

francescobrivio commented Jul 15, 2020

Sorry this was not supposed to be closed, I pressed the button by mistake. Should be re-opened now.

@francescobrivio
Copy link
Contributor Author

Hello,
Could you please provide a backport of this PR to 11_1_X for us (DQM) to test it in the playback system?
Regards,
Andrius

I will open the backport.

@jfernan2
Copy link
Contributor

@francescobrivio @gennai there were changes to this PR made by @andrius-k yesterday in the test done at the MWGR, as you suggested. We should include them in this PR, right? If we are expecting to make similar test in the future or actually use the code in online you should push these changes to the PR. Andrius can provide the diff if needed. Thanks

@cmsbuild
Copy link
Contributor

+1
Tested at: 1992d5b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-03e003/8045/summary.html
CMSSW: CMSSW_11_2_X_2020-07-16-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@francescobrivio
Copy link
Contributor Author

@francescobrivio @gennai there were changes to this PR made by @andrius-k yesterday in the test done at the MWGR, as you suggested. We should include them in this PR, right? If we are expecting to make similar test in the future or actually use the code in online you should push these changes to the PR. Andrius can provide the diff if needed. Thanks

I don't think this code is meant to be used in online (@gennai please correct me if I'm wrong), but we might want to use it again in similar tests. So if you can provide the diff I will push those changes as well.

@andrius-k
Copy link

Hi @francescobrivio,
I can see that you pushed another commit yesterday after we deployed the PR to P5. The diff between this PR (from the second commit) and what's currently running in P5 is the following:

diff --git a/CondCore/DBOutputService/src/OnlineDBOutputService.cc b/CondCore/DBOutputService/src/OnlineDBOutputService.cc
index 9f8065c5f44..c1ec99522e9 100644
--- a/CondCore/DBOutputService/src/OnlineDBOutputService.cc
+++ b/CondCore/DBOutputService/src/OnlineDBOutputService.cc
@@ -11,8 +11,9 @@ cond::service::OnlineDBOutputService::OnlineDBOutputService(const edm::Parameter
       m_preLoadConnectionString(iConfig.getUntrackedParameter<std::string>("preLoadConnectionString", "")),
       m_debug(iConfig.getUntrackedParameter<bool>("debugLogging", false)) {
   if (!m_lastLumiUrl.empty()) {
-    startTransaction();
-    m_runNumber = PoolDBOutputService::session().getCurrentRun().run;
+    //startTransaction();
+    //m_runNumber = PoolDBOutputService::session().getCurrentRun().run;
+    m_runNumber = iConfig.getUntrackedParameter<unsigned long long>("runNumber",100000);
   } else {
     m_lastLumiFile = iConfig.getUntrackedParameter<std::string>("lastLumiFile", "");
     //if( m_lastLumiFile.size() == 0 ){
diff --git a/DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py b/DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py
index 5cc6468f4a2..704ddcad8f6 100644
--- a/DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py
+++ b/DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py
@@ -34,6 +34,8 @@ noDB = True
 if 'noDB=False' in sys.argv:
     noDB=False

+noDB = False
+
 #---------------
 # Input sources
 if unitTest:
@@ -363,8 +365,8 @@ process.OnlineDBOutputService = cms.Service("OnlineDBOutputService",
     preLoadConnectionString = cms.untracked.string('frontier://FrontierProd/CMS_CONDITIONS'),

     runNumber = cms.untracked.uint64(options.runNumber),
-    lastLumiFile = cms.untracked.string('last_lumi.txt'),
-    #lastLumiUrl = cms.untracked.string('http://ru-c2e14-11-01.cms:11100/urn:xdaq-application:lid=52/getLatestLumiSection'),
+    #lastLumiFile = cms.untracked.string('last_lumi.txt'),
+    lastLumiUrl = cms.untracked.string('http://ru-c2e14-11-01.cms:11100/urn:xdaq-application:lid=52/getLatestLumiSection'),
     writeTransactionDelay = cms.untracked.uint32(options.transDelay),
     latency = cms.untracked.uint32(2),
     autoCommit = cms.untracked.bool(True),

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-03e003/8045/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2612943
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2612888
  • DQMHistoTests: Total skipped: 48
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
  • Checked 146 log files, 17 edm output root files, 35 DQM output files

@francescobrivio
Copy link
Contributor Author

Hi @francescobrivio,
I can see that you pushed another commit yesterday after we deployed the PR to P5. The diff between this PR (from the second commit) and what's currently running in P5 is the following:

diff --git a/CondCore/DBOutputService/src/OnlineDBOutputService.cc b/CondCore/DBOutputService/src/OnlineDBOutputService.cc
index 9f8065c5f44..c1ec99522e9 100644
--- a/CondCore/DBOutputService/src/OnlineDBOutputService.cc
+++ b/CondCore/DBOutputService/src/OnlineDBOutputService.cc
@@ -11,8 +11,9 @@ cond::service::OnlineDBOutputService::OnlineDBOutputService(const edm::Parameter
       m_preLoadConnectionString(iConfig.getUntrackedParameter<std::string>("preLoadConnectionString", "")),
       m_debug(iConfig.getUntrackedParameter<bool>("debugLogging", false)) {
   if (!m_lastLumiUrl.empty()) {
-    startTransaction();
-    m_runNumber = PoolDBOutputService::session().getCurrentRun().run;
+    //startTransaction();
+    //m_runNumber = PoolDBOutputService::session().getCurrentRun().run;
+    m_runNumber = iConfig.getUntrackedParameter<unsigned long long>("runNumber",100000);
   } else {
     m_lastLumiFile = iConfig.getUntrackedParameter<std::string>("lastLumiFile", "");
     //if( m_lastLumiFile.size() == 0 ){
diff --git a/DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py b/DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py
index 5cc6468f4a2..704ddcad8f6 100644
--- a/DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py
+++ b/DQM/Integration/python/clients/beamfake_dqm_sourceclient-live_cfg.py
@@ -34,6 +34,8 @@ noDB = True
 if 'noDB=False' in sys.argv:
     noDB=False

+noDB = False
+
 #---------------
 # Input sources
 if unitTest:
@@ -363,8 +365,8 @@ process.OnlineDBOutputService = cms.Service("OnlineDBOutputService",
     preLoadConnectionString = cms.untracked.string('frontier://FrontierProd/CMS_CONDITIONS'),

     runNumber = cms.untracked.uint64(options.runNumber),
-    lastLumiFile = cms.untracked.string('last_lumi.txt'),
-    #lastLumiUrl = cms.untracked.string('http://ru-c2e14-11-01.cms:11100/urn:xdaq-application:lid=52/getLatestLumiSection'),
+    #lastLumiFile = cms.untracked.string('last_lumi.txt'),
+    lastLumiUrl = cms.untracked.string('http://ru-c2e14-11-01.cms:11100/urn:xdaq-application:lid=52/getLatestLumiSection'),
     writeTransactionDelay = cms.untracked.uint32(options.transDelay),
     latency = cms.untracked.uint32(2),
     autoCommit = cms.untracked.bool(True),

thanks @andrius-k !
I prefer not to touch the CondCore package, since @ggovi wrote that particular module and he might want to work more on that.
About the fake beamspot clients:

  • the lastLumiFile/Url change has alrady been implemented in: 1992d5b
  • the parameter noDB = False instead should be read from command line, was there a problem with that?

@andrius-k
Copy link

Hi @francescobrivio
No problem!
Regarding the noDB argument, unfortunately we can't selectively pass in different arguments to different Online clients. Is it possible for you to set the default value to whatever you would like it to be when running in Online production?

@jfernan2
Copy link
Contributor

@francescobrivio @ggovi could you clarify a but about the noDB parameter, in particular:

  • we cannot run selectively as @andrius-k said, except for limited tests for which we understand the FakeBeamMonitor is intended for, at least in some circustances
  • for MWGRs, we understand that noDB should be then equal to False, i.e. the DB will be accesible, a run and a LS are available from DAQ, is this correct?
  • noDB = True should be used for "offline" tests with no live mode or UnitTest mode, or any other playback mode to not overwrite the DB?
    Thanks

@silviodonato
Copy link
Contributor

kind reminder @francescobrivio

@gennai
Copy link
Contributor

gennai commented Jul 27, 2020

Hi, I can answer on the above points. noDB means that a sql file is created instead of uploading the new payload to the Prod condition DB. You are basically right.
noDB= True should be used for unit test and other local tests (including playback). For MWGR noDB must be = False as we do want to upload the payload to the database.

@francescobrivio
Copy link
Contributor Author

Sorry for the delay. Please see the inline answers.

@francescobrivio @ggovi could you clarify a but about the noDB parameter, in particular:

* we cannot run selectively as @andrius-k said, except for limited tests for which we understand the FakeBeamMonitor is intended for, at least in some circustances

Not sure I understand the run selectively part. Can't this option be bassed from command line?

* for MWGRs, we understand that noDB should be then equal to False, i.e. the DB will be accesible, a run and a LS are available from DAQ, is this correct?

This is correct

* noDB = True should be used for "offline" tests with no live mode or UnitTest mode, or any other playback mode to not overwrite the DB?

Correct.

The noDB = False option (access the DB and upload conditions) should only be used for specific tests (e.g. during MWGR) or in normal data taking periods, as you can see in the "normal" BeamSpot client here

@jfernan2
Copy link
Contributor

jfernan2 commented Jul 27, 2020

Thanks @francescobrivio @gennai
So, the default config we want is noDB = True to avoid real uploading (as it is the PR right now).

noDB = False will only be used on very dedicated ocasions, since DQM core will have to pass it manually by command line on a very limited number of runs. We have to manually switch it off on the client running at P5, which is error prone and not desired, that's what I meant by run selectively

Correct?

@gennai
Copy link
Contributor

gennai commented Jul 27, 2020

Thanks @francescobrivio @gennai
So, the default config we want is noDB = True to avoid real uploading (as it is the PR right now).

noDB = False will only be used on very dedicated ocasions, since DQM core will have to pass it manually by command line on a very limited number of runs. We have to manually switch it off on the client running at P5, which is error prone and not desired, that's what I meant by run selectively

Correct?

yes.

@jfernan2
Copy link
Contributor

+1

@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)

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 2e95b4e into cms-sw:master Jul 27, 2020
@jfernan2
Copy link
Contributor

@francescobrivio we need a new PR in Master with
lastLumiFile = cms.untracked.string('last_lumi.txt'),
to make this PR working out of the box as commented in
#30696 (comment)
Thanks

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.

6 participants