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

BeamSpot DQM clients modified to log to onlineDB only when beam status from TCDS is STABLE #37614

Merged
merged 3 commits into from Apr 23, 2022

Conversation

dzuolo
Copy link
Contributor

@dzuolo dzuolo commented Apr 19, 2022

Following discussion with @ggovi BeamSpot DQM clients have been modified in such a way that they log to onlineDB only when the beam mode from TCDS is STABLE. This prevents the generation of dummy logs containing only "fit failed" reports during, e.g., cosmics data taking.

Code in this PR compile and the following local tests were successful

-scram b code-format
-scram b code-checks
-scramv1 b runtests

This PR is not a backport and a backport to 12_3 will provided shortly.

FYI: @gennai @francescobrivio

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37614/29385

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @dzuolo (Davide Zuolo) for master.

It involves the following packages:

  • DQM/BeamMonitor (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@jfernan2
Copy link
Contributor

test parameters:

  • addpkg = DQM/Integration

@dzuolo
Copy link
Contributor Author

dzuolo commented Apr 19, 2022

@cmsbuild please test

@dzuolo
Copy link
Contributor Author

dzuolo commented Apr 19, 2022

Apparently I am not an "authorised" user that can issue the test command so if somebody could it for me I would appreciate it :)

@mmusich
Copy link
Contributor

mmusich commented Apr 19, 2022

please test

Copy link
Contributor

@mmusich mmusich left a comment

Choose a reason for hiding this comment

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

minor comment

DQM/BeamMonitor/plugins/BeamMonitor.cc Outdated Show resolved Hide resolved
@dzuolo dzuolo changed the title BeamSpot DQM clients modified to log to onlineDB only when beam statu… BeamSpot DQM clients modified to log to onlineDB only when beam status from TCDS is STABLE Apr 19, 2022
@francescobrivio
Copy link
Contributor

I see two errors in the unitTests:

  1. related to the beam_dqm_sourceclient
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 344518 lumi: 2 event: 31571 stream: 0
   [1] Running path 'p'
   [2] Calling method for module BeamMonitor/'dqmBeamMonitor'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: TCDSRecord
Looking for module label: tcdsDigis
Looking for productInstanceName: tcdsRecord

which is exactly what what is touched in this PR. It seems like tcdsRecord is not present in the data? I'm not totally sure...

  1. related to the l1tstage2_dqm_sourceclient which is completely unrelated to this PR.

@jfernan2
Copy link
Contributor

2nd error is known and being discussed here:
#37584

@mmusich
Copy link
Contributor

mmusich commented Apr 19, 2022

which is exactly what what is touched in this PR. It seems like tcdsRecord is not present in the data? I'm not totally sure...

You need to unpack it...

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7ac7d2/24006/summary.html
COMMIT: fa33b11
CMSSW: CMSSW_12_4_X_2022-04-19-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37614/24006/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test TestDQMOnlineClient-beam_dqm_sourceclient had ERRORS
---> test TestDQMOnlineClient-l1tstage2_dqm_sourceclient had ERRORS

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3589937
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3589913
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 200 log files, 45 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor

mmusich commented Apr 20, 2022

You need to unpack it...

just to clarify on the comment which might appear a bit terse, what I meant is that the TCDS unpacker needs to be included in the path run in the online client, just as it was done for the onlineMetaData digis here, see e.g. here:

from EventFilter.OnlineMetaDataRawToDigi.tcdsRawToDigi_cfi import *
process.tcdsDigis = tcdsRawToDigi.clone()

@dzuolo
Copy link
Contributor Author

dzuolo commented Apr 20, 2022

If I understood correctly the failed unit test is not related to this PR and is being discussed in #37584. No other errors popped up.

@francescobrivio
Copy link
Contributor

I think you need to modify also beamhlt_dqm_sourceclient-live_cfg.py which calls the same plugin (BeamMonitor).

apparently it's not tested (that's why it didn't fail..)

<!-- Need an HLT stream -->
<!-- <test name="TestDQMOnlineClient-beamhlt_dqm_sourceclient" command="runtest.sh beamhlt_dqm_sourceclient-live_cfg.py" /> -->

given we somehow expect it to be the workhorse for Run3, can we find a way of giving it an HLT-stream like input? @francescobrivio @cms-sw/dqm-l2

Ok I think I have a working solution for this...I just need to get hold of a streamer file for streamDQMOnlineBeamspot stream. 😂
Once I get that from DQM team I'll open a different PR to add the proper unit test.

@francescobrivio
Copy link
Contributor

@emanueleusai given that you signed the 12_3_X backport can you sign this one as well?
The tests are clean except the known failure with TestDQMOnlineClient-l1tstage2_dqm_sourceclient (see #37584)

@emanueleusai
Copy link
Member

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

please test
(a temporary fix for the TestDQMOnlineClient-l1tstage2_dqm_sourceclient failure was made available in the IBs with #37634)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7ac7d2/24143/summary.html
COMMIT: 691485e
CMSSW: CMSSW_12_4_X_2022-04-22-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37614/24143/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-7ac7d2/39434.75_TTbar_14TeV+2026D88_HLT75e33+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HLT75e33+HARVESTGlobal

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3695434
  • DQMHistoTests: Total failures: 14
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3695398
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 205 log files, 45 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor

+1

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