Test ECAL pedestals in next MWGR #33765

francescobrivio · 2021-05-18T14:57:23Z

PR description:

In order to test the ECAL pedestal workflow in PCL during next MWGR we need to change
the flag requireStableBeam in ecalPedestalPCLworker_cfi.py, because usually the ECAL pedestal
PCL workflow is only run with collisions (with stable beams to be precise).

Since this change is needed only for the upcoming MWGR and after that it will be reverted I'm not sure
if there is a smarter way to configure this without the need for a patch release, I'm open to suggestions.

ECAL DPGs are informed and agree with this change provided we revert it after the MWGR.

PR validation:

Tested with RunTheMatrix.py -l 1010 --ibeos

Backport:

This is not a backport and no backport is needed. Will be reverted after MWGR.

cmsbuild · 2021-05-18T14:57:48Z

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

It involves the following packages:

Calibration/EcalCalibAlgos

@malbouis, @yuanchao, @cmsbuild, @tlampen, @pohsun, @francescobrivio can you please review it and eventually sign? Thanks.
@rchatter, @tocheng, @argiro, @thomreis, @simonepigazzini, @mmusich 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

francescobrivio · 2021-05-18T15:00:34Z

test parameters:

workflows = 1010.0

francescobrivio · 2021-05-18T15:01:26Z

@cmsbuild please test

francescobrivio · 2021-05-18T15:04:30Z

FYI @amassiro @boudoul

mmusich · 2021-05-18T15:16:54Z

Calibration/EcalCalibAlgos/python/ecalPedestalPCLworker_cfi.py

@@ -5,7 +5,7 @@
                                BarrelDigis=cms.InputTag('ecalDigis','ebDigis'),
                                EndcapDigis=cms.InputTag('ecalDigis','eeDigis'),
                                tcdsRecord  =cms.InputTag('tcdsDigis','tcdsRecord'),
-                                requireStableBeam = cms.bool(True),
+                                requireStableBeam = cms.bool(False),


wildly speculating: can this be included in the cosmics reconstruction scenario, which by construction doesn't have stable beams, without having to pollute conditions with unstable collisions?
https://github.com/cms-sw/cmssw/blob/master/Configuration/DataProcessing/python/Impl/cosmics.py
Alternatively one can adapt the C++ source code here:

cmssw/Calibration/EcalCalibAlgos/src/ECALpedestalPCLworker.cc

Lines 48 to 54 in c861ecf

if (requireStableBeam_) {

edm::Handle<TCDSRecord> tcdsData;

iEvent.getByToken(tcdsToken_, tcdsData);

int beamMode = tcdsData->getBST().getBeamMode();

if (beamMode != BSTRecord::BeamMode::STABLE)

return;

}

to accept also BSTRecord::BeamMode::NOBEAM (I suppose that's what the TDSC record has for cosmics).

Not sure though one wants to run the ecal pedestal on all cosmics runs.

From a private conversation with @simonepigazzini I understood they are fine running this test in the upcoming MWGR, but otherwise they would like to keep things as they were, i.e. only run the pedestals with stable beams.
So I don't think including this in the cosmic scenario is the way to go. Maybe create a MWGR scenario? (if this even makes sense...)

I don't see how your code proposal differs from mine in this respect other than being worse, since it also accepts the collision in unstable beam.

Ah I see your point now...indeed it might be a good solution to run it only for cosmics and it can still be reverted after the MWGR. I'll have a look how to customize the cosmics.py.

I understood they are fine running this test in the upcoming MWGR, but otherwise they would like to keep things as they were, i.e. only run the pedestals with stable beams.

One additional comment.
If there is concern that running the ECAL pedestal workflow on non-stable beams might produce conditions polluting the production pedestal tags, a valid option is to make this line configurable:

cmssw/Calibration/EcalCalibAlgos/src/ECALpedestalPCLHarvester.cc

Line 147 in c861ecf

poolDbService->writeOne(&pedestals, poolDbService->currentTime(), "EcalPedestalsRcd");

so that in the case of non-stable beam configuration it writes on a different record name than EcalPedestalsRcd (can be whatever random string) and then use that key to steer the PCL upload to a different tag name at the level of DropBoxMetaData by adding an appropriate entry here (for a different record).

cmssw/CondFormats/Common/test/ProduceDropBoxMetadata.py

Lines 145 to 149 in 74144ae

cms.PSet(record = cms.untracked.string('EcalPedestalsRcd'),

Source = cms.untracked.string("AlcaHarvesting"),

FileClass = cms.untracked.string("ALCA"),

prodMetaData = cms.untracked.string(EcalPedestalsRcd_prod_str),

prepMetaData = cms.untracked.string(EcalPedestalsRcd_prep_str),

Notice that in this context a record is just a dummy string and doesn't map to the actual C++ class associated to the CondFormat (as you can see in the DropBoxMetaData we have all sort of dummy names).

@mmusich two further cosiderations:

Regarding the flag: since this is a one time only change (@simonepigazzini please confirm this from ECAL side) and will be reverted after MWGR I think it's faster to change the flag as I already did instead of modifying the cosmics customization (because I need some time to understand how to do that properly).

In any case, in order not to pollute the production tags, we are planning to write the payloads to a test tag as you suggest. I was under the impression that changing the DropBoxMetaData was enough to do that, but from your last comment it seems that some hardcoded changes are needed in

cmssw/Calibration/EcalCalibAlgos/src/ECALpedestalPCLHarvester.cc

Line 147 in c861ecf

poolDbService->writeOne(&pedestals, poolDbService->currentTime(), "EcalPedestalsRcd");

Could you help me understand better?

@francescobrivio the DropBoxMetaData mechanism assigns the tags on which to write the payload produced by the PCL by means of reading from the framework job report the target record produced by the PCL harvester configuration. That's why the record in the source C++ code is relevant to steer which tags to populate. If you don't foresee to have a separate harvesting for cosmics then there is no need to change the source code (that would be needed to maintain at the same time two harvesting flavours), but just create a new IOV for the DropBoxMetaData record in the express Global Tag and then roll it back after the MWGR.
We can talk more at length offline if you wish.

@francescobrivio the DropBoxMetaData mechanism assigns the tags on which to write the payload produced by the PCL by means of reading from the framework job report the target record produced by the PCL harvester configuration. That's why the record in the source C++ code is relevant to steer which tags to populate. If you don't foresee to have a separate harvesting for cosmics then there is no need to change the source code (that would be needed to maintain at the same time two harvesting flavours),

Thanks for the nice explanation.

but just create a new IOV for the DropBoxMetaData record in the express Global Tag and then roll it back after the MWGR.

Yes, this is what I had in mind to do.

We can talk more at length offline if you wish.

Yes, that would be really useful thanks!

So I think this PR is fine as it is now, given also that all the tests are succesful.

cmsbuild · 2021-05-18T21:39:18Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6764c3/15167/summary.html
COMMIT: 0d240fa
CMSSW: CMSSW_11_3_X_2021-05-18-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33765/15167/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-6764c3/1010.0_TestEnableEcalHCAL2017B+TestEnableEcalHCAL2017B+TIER0EXPTE+ALCAEXPTE+ALCAHARVDTE

Summary:

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

malbouis · 2021-05-20T14:10:00Z

+alca

The flag changed in this PR will allow the ECAL pedestal PCL to be tested during the upcoming MWGR. It will be immediately reverted after the MWGR and in principle there are no intentions on repeating this exercise with this particular workflow.

cmsbuild · 2021-05-20T14:10:22Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_11_3_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_0_X is complete. 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 · 2021-05-21T08:05:55Z

+1
cc @boudoul @amassiro

change ECAL pedestals requireStableBeam flag for MWGR test

0d240fa

cmsbuild added this to the CMSSW_11_3_X milestone May 18, 2021

cmsbuild added alca-pending orp-pending pending-signatures tests-pending labels May 18, 2021

cmsbuild added tests-started and removed tests-pending labels May 18, 2021

mmusich reviewed May 18, 2021

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels May 18, 2021

francescobrivio changed the title ~~[RFC] Test ECAL pedestals in next MWGR~~ Test ECAL pedestals in next MWGR May 20, 2021

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels May 20, 2021

cmsbuild added orp-approved and removed orp-pending labels May 21, 2021

cmsbuild merged commit d07a829 into cms-sw:CMSSW_11_3_X May 21, 2021

francescobrivio mentioned this pull request May 31, 2021

Replay test with CMSSW_11_3_1 dmwm/T0#4574

Closed

francescobrivio deleted the alca-ECALpedestalsMWGR_11_3_X branch June 16, 2021 22:07

francescobrivio mentioned this pull request Jun 17, 2021

Update GEMRECO geometry in online GTs [11_3_X] #34166

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test ECAL pedestals in next MWGR #33765

Test ECAL pedestals in next MWGR #33765

francescobrivio commented May 18, 2021

cmsbuild commented May 18, 2021

francescobrivio commented May 18, 2021

francescobrivio commented May 18, 2021

francescobrivio commented May 18, 2021

mmusich May 18, 2021

francescobrivio May 18, 2021

mmusich May 18, 2021 •

edited

francescobrivio May 18, 2021

mmusich May 18, 2021

francescobrivio May 19, 2021

mmusich May 19, 2021

francescobrivio May 19, 2021

cmsbuild commented May 18, 2021

malbouis commented May 20, 2021

cmsbuild commented May 20, 2021

silviodonato commented May 21, 2021

	if (requireStableBeam_) {
	edm::Handle<TCDSRecord> tcdsData;
	iEvent.getByToken(tcdsToken_, tcdsData);
	int beamMode = tcdsData->getBST().getBeamMode();
	if (beamMode != BSTRecord::BeamMode::STABLE)
	return;
	}

	cms.PSet(record = cms.untracked.string('EcalPedestalsRcd'),
	Source = cms.untracked.string("AlcaHarvesting"),
	FileClass = cms.untracked.string("ALCA"),
	prodMetaData = cms.untracked.string(EcalPedestalsRcd_prod_str),
	prepMetaData = cms.untracked.string(EcalPedestalsRcd_prep_str),

Test ECAL pedestals in next MWGR #33765

Test ECAL pedestals in next MWGR #33765

Conversation

francescobrivio commented May 18, 2021

PR description:

PR validation:

Backport:

cmsbuild commented May 18, 2021

francescobrivio commented May 18, 2021

francescobrivio commented May 18, 2021

francescobrivio commented May 18, 2021

mmusich May 18, 2021

Choose a reason for hiding this comment

francescobrivio May 18, 2021

Choose a reason for hiding this comment

mmusich May 18, 2021 • edited

Choose a reason for hiding this comment

francescobrivio May 18, 2021

Choose a reason for hiding this comment

mmusich May 18, 2021

Choose a reason for hiding this comment

francescobrivio May 19, 2021

Choose a reason for hiding this comment

mmusich May 19, 2021

Choose a reason for hiding this comment

francescobrivio May 19, 2021

Choose a reason for hiding this comment

cmsbuild commented May 18, 2021

Comparison Summary

malbouis commented May 20, 2021

cmsbuild commented May 20, 2021

silviodonato commented May 21, 2021

mmusich May 18, 2021 •

edited