add onlineBeamSpotESProducer to BeamSpot_cfi: fix general Online BS swap case #35639

mmusich · 2021-10-13T08:49:14Z

resolves #35634

PR description:

In PR #35373 the general case for the online <-> offline BS swap done via modifiers was missed, leading to issues in the visualization DQM clients.
This PR supplies the onlineBeamSpotESProducer to BeamSpot_cfi as a general case fix-up.

PR validation:

Run cmsRun DQM/Integration/python/clients/visualization-live_cfg.py unitTest=True
that before was crashing with:

...
Exception Message:
No "BeamSpotTransientObjectsRcd" record found in the EventSetup.n
 Please add an ESSource or ESProducer that delivers such a record.
----- End Fatal Exception -------------------------------------------------

while now crashes differently with:

----- Begin Fatal Exception 13-Oct-2021 10:41:57 CEST-----------------------
An exception of category 'NoProxyException' occurred while
   [0] Processing  Event run: 334393 lumi: 1 event: 16132 stream: 1
   [1] Running path 'FEVToutput_step'
   [2] Prefetching for module JsonWritingTimeoutPoolOutputModule/'FEVToutput'
   [3] Prefetching for module ReducedRecHitCollectionProducer/'reducedEcalRecHitsEB'
   [4] Prefetching for module EleIsoDetIdCollectionProducer/'interestingGedEleIsoDetIdEB'
   [5] Calling method for module GEDGsfElectronFinalizer/'gedGsfElectrons'
Exception Message:
No data of type "GBRForestD" with label "electron_eb_ecalOnly_1To300_0p2To2_mean" in record "GBRDWrapperRcd"
 Please add an ESSource or ESProducer to your job which can deliver this data.
----- End Fatal Exception -------------------------------------------------

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

Not a backport, but should be backported.

…wap case

cmsbuild · 2021-10-13T08:56:13Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35639/25912

This PR adds an extra 12KB to repository

cmsbuild · 2021-10-13T08:56:36Z

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

RecoVertex/BeamSpotProducer (reconstruction, alca)

@malbouis, @yuanchao, @cmsbuild, @slava77, @jpata, @tvami, @francescobrivio can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @tocheng, @mmusich, @mtosi, @dgulhan, @francescobrivio this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

francescobrivio · 2021-10-13T08:59:29Z

RecoVertex/BeamSpotProducer/python/BeamSpot_cfi.py

@@ -4,6 +4,11 @@

 offlineBeamSpot = cms.EDProducer("BeamSpotProducer")

+import RecoVertex.BeamSpotProducer.onlineBeamSpotESProducer_cfi as _mod
+BeamSpotESProducer = _mod.onlineBeamSpotESProducer.clone(
+    timeThreshold = 999999 # for express allow >48h old payloads for replays. DO NOT CHANGE


Does this mean that wherever this module is imported it gets this timeThreshold? I don't think this is the intended use for the HLT case, right @gennai ?

Isn't HLT fully dumped anyway?

mmusich · 2021-10-13T09:04:34Z

please test

gennai · 2021-10-13T09:08:02Z

I am not even sure it would work for importing the beamspotProducer in the menu. I do not know if confDB can ignore the part not related to the module description, @Sam-Harper may know better than I do.

gennai · 2021-10-13T09:09:23Z

or even @Martin-Grunewald may know the answer

Martin-Grunewald · 2021-10-13T09:45:11Z

What is the actual question?

The beam spot producer currently in 12_1 HLT offline/development menus is the BeamSpotOnlineProducer configured like this

hltOnlineBeamSpot = cms.EDProducer( "BeamSpotOnlineProducer",
    changeToCMSCoordinates = cms.bool( False ),
    maxZ = cms.double( 40.0 ),
    setSigmaZ = cms.double( 0.0 ),
    beamMode = cms.untracked.uint32( 11 ),
    src = cms.InputTag( "hltScalersRawToDigi" ),
    gtEvmLabel = cms.InputTag( "" ),
    maxRadius = cms.double( 2.0 ),
    useTransientRecord = cms.bool( False )
)

There is no parameter called timeThreshold so this would be set via fillDescriptions, or lead to a crash if not.

mmusich · 2021-10-13T09:48:52Z

There is no parameter called timeThreshold so this would be set via fillDescriptions, or lead to a crash if not.

Indeed it is:

cmssw/RecoVertex/BeamSpotProducer/plugins/OnlineBeamSpotESProducer.cc

Line 61 in df9a144

dsc.add<int>("timeThreshold", 48)->setComment("hours");

and that's what we want for HLT, no? @gennai @francescobrivio

Martin-Grunewald · 2021-10-13T09:49:06Z

Perhaps you talk about CRUZET/CRAFT menus by FOG?

Martin-Grunewald · 2021-10-13T09:51:28Z

If you want to replace the BeamSpotOnlineProducer by an OnlineBeamSpotESProducer, in the offline/development menus then we need to know (JIRA ticket or similar). It may have already happened in the menus currently online... but there is no feedback to the development menus offline so far @gennai

In ConfDB the following is available for use:

OnlineBeamSpotESProducer = cms.ESProducer( "OnlineBeamSpotESProducer",
    timeThreshold = cms.int32( 48 ),
    sigmaZThreshold = cms.double( 2.0 ),
    appendToDataLabel = cms.string( "" )
)

Changing the timeThreshold parameter in the cfi would not affect HLT...

francescobrivio · 2021-10-13T11:33:03Z

There is no parameter called timeThreshold so this would be set via fillDescriptions, or lead to a crash if not.

Indeed it is:

cmssw/RecoVertex/BeamSpotProducer/plugins/OnlineBeamSpotESProducer.cc

Line 61 in df9a144

dsc.add<int>("timeThreshold", 48)->setComment("hours");

and that's what we want for HLT, no? @gennai @francescobrivio

Correct, 48h is what we want!

francescobrivio · 2021-10-13T11:34:52Z

In ConfDB the following is available for use:
OnlineBeamSpotESProducer = cms.ESProducer( "OnlineBeamSpotESProducer",
    timeThreshold = cms.int32( 48 ),
    sigmaZThreshold = cms.double( 2.0 ),
    appendToDataLabel = cms.string( "" )
)
Changing the timeThreshold parameter in the cfi would not affect HLT...

Thanks @Martin-Grunewald that's perfect! We do not want to change any of those parameters in the HLT, so the configuration from ConfDB that you reported is the correct one to be used.

gennai · 2021-10-13T12:14:52Z

Sorry everybody I was unclear. The ESProducer is already in the menu as well as the OnlineBeamSpotProducer with the proper configuration and we do not want to change it.

I was referring to the fact that the proposal fix is adding few lines to a cfi file adding on top of the OnlineBeamSpotProducer configuration also an import of the ESProducer module with a change in one parameter.

I was questioning (mostly @Martin-Grunewald) if the practice of adding other modules in a cfi file could have given issues in future migration of the DB templates to later releases. IF it is not the case I am fine with the fix.

cmsbuild · 2021-10-13T12:22:32Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-30d3c1/19583/summary.html
COMMIT: 9870cee
CMSSW: CMSSW_12_1_X_2021-10-12-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35639/19583/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 2798082
DQMHistoTests: Total failures: 10
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2798049
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 39 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

slava77 · 2021-10-15T12:29:11Z

test parameters:

addpkg = DQM/Integration,Configuration/DataProcessing
pull_request = [Online DQM] re-introduce unit tests for visualization #35642

slava77 · 2021-10-15T12:29:29Z

@cmsbuild please test

tvami · 2021-10-15T13:32:40Z

urgent

slava77 · 2021-10-15T14:58:21Z

+reconstruction

for #35639 d6091aa

code changes are in line with the PR description and the follow up review
- we should revisit in a follow up how to make the express settings more uniform between the data processing scenarios and the cmsDriver
jenkins tests pass and comparisons with the baseline show no (relevant) differences

tvami · 2021-10-15T15:00:18Z

+alca

cmsbuild · 2021-10-15T15:00:45Z

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

RecoVertex/BeamSpotProducer/python/BeamSpot_cfi.py

perrotta · 2021-10-15T16:17:06Z

+1

perrotta · 2021-10-15T16:19:55Z

merge

cmsbuild · 2021-10-15T18:35:14Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-30d3c1/19656/summary.html
COMMIT: d6091aa
CMSSW: CMSSW_12_1_X_2021-10-14-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35639/19656/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 2769241
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2769219
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 170 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

add onlineBeamSpotESProducer to BeamSpot_cfi: fix general Online BS s…

9870cee

…wap case

cmsbuild added this to the CMSSW_12_1_X milestone Oct 13, 2021

cmsbuild added alca-pending code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Oct 13, 2021

mmusich mentioned this pull request Oct 13, 2021

Crash in visualization DQM clients during SPLASH test #35634

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 13, 2021

francescobrivio reviewed Oct 13, 2021

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Oct 13, 2021

mmusich mentioned this pull request Oct 13, 2021

[Online DQM] re-introduce unit tests for visualization #35642

Merged

cmsbuild added tests-approved and removed tests-started labels Oct 13, 2021

cmsbuild added tests-pending and removed tests-started labels Oct 15, 2021

cmsbuild added requires-external tests-started and removed tests-pending labels Oct 15, 2021

cmsbuild added the urgent label Oct 15, 2021

mmusich mentioned this pull request Oct 15, 2021

provide a reconstruction scenario for the commissioning_run type introduced in #34832 #35691

Merged

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Oct 15, 2021

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Oct 15, 2021

slava77 mentioned this pull request Oct 15, 2021

harmonize express config settings between DataProcessing scenarios and cmsDriver #35693

Open

perrotta reviewed Oct 15, 2021

View reviewed changes

RecoVertex/BeamSpotProducer/python/BeamSpot_cfi.py Show resolved Hide resolved

cmsbuild added orp-approved and removed orp-pending labels Oct 15, 2021

cmsbuild merged commit 5b230af into cms-sw:master Oct 15, 2021

mmusich mentioned this pull request Oct 15, 2021

fix typo in _loadOnlineBeamSpotESProducer #35696

Merged

cmsbuild added tests-approved and removed tests-started labels Oct 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add onlineBeamSpotESProducer to BeamSpot_cfi: fix general Online BS swap case #35639

add onlineBeamSpotESProducer to BeamSpot_cfi: fix general Online BS swap case #35639

mmusich commented Oct 13, 2021

cmsbuild commented Oct 13, 2021

cmsbuild commented Oct 13, 2021

francescobrivio Oct 13, 2021 •

edited

Loading

mmusich Oct 13, 2021

mmusich commented Oct 13, 2021

gennai commented Oct 13, 2021

gennai commented Oct 13, 2021

Martin-Grunewald commented Oct 13, 2021 •

edited

Loading

mmusich commented Oct 13, 2021 •

edited

Loading

Martin-Grunewald commented Oct 13, 2021

Martin-Grunewald commented Oct 13, 2021 •

edited

Loading

francescobrivio commented Oct 13, 2021

francescobrivio commented Oct 13, 2021

gennai commented Oct 13, 2021

cmsbuild commented Oct 13, 2021

slava77 commented Oct 15, 2021

slava77 commented Oct 15, 2021

tvami commented Oct 15, 2021

slava77 commented Oct 15, 2021

tvami commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

perrotta commented Oct 15, 2021

perrotta commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

add onlineBeamSpotESProducer to BeamSpot_cfi: fix general Online BS swap case #35639

add onlineBeamSpotESProducer to BeamSpot_cfi: fix general Online BS swap case #35639

Conversation

mmusich commented Oct 13, 2021

PR description:

PR validation:

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

cmsbuild commented Oct 13, 2021

cmsbuild commented Oct 13, 2021

francescobrivio Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

mmusich Oct 13, 2021

Choose a reason for hiding this comment

mmusich commented Oct 13, 2021

gennai commented Oct 13, 2021

gennai commented Oct 13, 2021

Martin-Grunewald commented Oct 13, 2021 • edited Loading

mmusich commented Oct 13, 2021 • edited Loading

Martin-Grunewald commented Oct 13, 2021

Martin-Grunewald commented Oct 13, 2021 • edited Loading

francescobrivio commented Oct 13, 2021

francescobrivio commented Oct 13, 2021

gennai commented Oct 13, 2021

cmsbuild commented Oct 13, 2021

Comparison Summary

slava77 commented Oct 15, 2021

slava77 commented Oct 15, 2021

tvami commented Oct 15, 2021

slava77 commented Oct 15, 2021

tvami commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

perrotta commented Oct 15, 2021

perrotta commented Oct 15, 2021

cmsbuild commented Oct 15, 2021

Comparison Summary

francescobrivio Oct 13, 2021 •

edited

Loading

Martin-Grunewald commented Oct 13, 2021 •

edited

Loading

mmusich commented Oct 13, 2021 •

edited

Loading

Martin-Grunewald commented Oct 13, 2021 •

edited

Loading