Select the unpacker to be run (between SCAL and MetaData) via Era modifier #27676

arossi83 · 2019-08-02T11:26:00Z

PR description:

Actually both unpacker (for Scalers and MetaData) are inserted in the RawToDigi sequence. With the changes proposed here the OnlineMetaData unpacker is inserted only starting from Era Run2_2018 (when these data start to be available). After these changes all the MetaData/SCAL consumers can simply select the available Handle exploiting isValid method in order to preserve backward compatibility of the code.

(with the same approach Scalers unpacker could be removed starting from Run3, if needed)

PR validation:

Manually tested on a run from 2017(no softFED1022) and one from 2018(with softFED1022).

2017
cmsDriver.py step1 -s RAW2DIGI,L1Reco,RECO,DQM -n 100 --eventcontent DQM --datatier DQMIO --conditions 110X_dataRun2_v1 --filein /store/data/Run2018D/ZeroBias/RAW/v1/000/323/997/00000/DDE05469-39F3-0647-BC31-AA8247C0B6C1.root --data --no_exec --python_filename=runFirst2017.py --era Run2_2017
cmsRun runFirst2017.py
2018
cmsDriver.py step1 -s RAW2DIGI,L1Reco,RECO,DQM -n 100 --eventcontent DQM --datatier DQMIO --conditions 110X_dataRun2_v1 --filein /store/data/Run2018D/ZeroBias/RAW/v1/000/323/997/00000/DDE05469-39F3-0647-BC31-AA8247C0B6C1.root --data --no_exec --python_filename=runFirst2018.py --era Run2_2018
cmsRun runFirst2018.py

if this PR is a backport please specify the original PR:

is not a backport

…aDataDigis) via Era modifier

cmsbuild · 2019-08-02T11:26:28Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-08-02T11:32:11Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27676/11262

This PR adds an extra 20KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/StandardSequences/python/RawToDigi_cff.py modified in PR(s): Complete the implementation of RPC Digi Collection Merger started in PR #24613 #27672

cmsbuild · 2019-08-02T11:32:41Z

A new Pull Request was created by @arossi83 (Alessandro Rossi) for master.

It involves the following packages:

Configuration/Eras
Configuration/StandardSequences

@cmsbuild, @franzoni, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @dgulhan this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

arossi83 · 2019-08-02T11:35:40Z

i.e. This changes make possible to easily switch between OnlineMetaData and LumiScalers like proposed on PR #27677 for TrackingDQM

fioriNTU · 2019-08-02T14:15:21Z

@arossi83 why you didn't include this pr as a commit on #27677 ? Can you clarify?

mmusich · 2019-08-02T14:23:34Z

@fioriNTU, #27677 is very-DQM specific, while this one is more concerning the "general" approach and needs to be agreed upon by RECO and DAQ (as we discussed during the last (RECO/AT meeting).
#27677 can be tested meaningfully only after this one is merged. If we choose this approach and this PR is merged other PRs in the style of #27677 will follow.
An alternative approach it the one followed in this PR #27638 that doesn't require to change the raw2digi sequence.

fabiocos · 2019-08-05T12:31:07Z

@mmusich @perrotta @slava77 I did not attend that part of the meeting, what was the outcome of the discussion about the issue that this PR is addressing?

fabiocos · 2019-08-05T12:31:26Z

please test

cmsbuild · 2019-08-05T12:31:34Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1833/console Started: 2019/08/05 14:32

mmusich · 2019-08-05T12:47:32Z

I did not attend that part of the meeting, what was the outcome of the discussion about the issue that this PR is addressing?

@fabiocos we proposed the solution implemented in this PR that was not met with any negative feedback from the audience, though if there is a better / preferred way, of course a different strategy can be used. It would be important though that it is a generally agreed upon solution (hence the request to discuss at the RECO/AT meeting) as it has consequences on how the migration is carried out from several subsystems.
cc @mtosi

makortel · 2019-08-05T14:39:07Z

Let me repeat/rephrase my comment #27677 (comment) that for the framework it is more natural (and thus more performant) to customize what is being consumed rather than what is being produced (including a per-event check of what is available in the event), because the (unscheduled) execution is driven by the consumes dependencies.

(sorry for not being able to attend the meeting)

cmsbuild · 2019-08-05T15:09:05Z

-1

Tested at: f1ccf28

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-35450d/1833/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests

Unit Tests:

I found errors in the following unit tests:

---> test TestConfigDP had ERRORS

cmsbuild · 2019-08-05T15:09:08Z

Comparison job queued.

cmsbuild · 2019-08-05T17:20:58Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-35450d/1833/summary.html

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

/build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-35450d/11634.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2021_GenSimFull+DigiFull_2021+RecoFull_2021+HARVESTFull_2021+ALCAFull_2021

Comparison Summary:

No significant changes to the logs found
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 33
DQMHistoTests: Total histograms compared: 2715989
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2715659
DQMHistoTests: Total skipped: 329
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
Checked 137 log files, 14 edm output root files, 33 DQM output files

cmsbuild · 2019-08-08T10:53:49Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1904/console Started: 2019/08/08 12:54

cmsbuild · 2019-08-08T13:22:40Z

+1
Tested at: f1ccf28
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-35450d/1904/summary.html

cmsbuild · 2019-08-08T13:22:43Z

Comparison job queued.

slava77 · 2019-08-08T15:25:57Z

Configuration/Eras/python/Era_Run2_2018_cff.py

-Run2_2018 = cms.ModifierChain(Run2_2017.copyAndExclude([run2_HEPlan1_2017, run2_muon_2017, run2_L1prefiring, run2_HLTconditions_2017, run2_egamma_2017, ctpps_2017]),
-run2_CSC_2018, run2_HCAL_2018, run2_HB_2018, run2_HE_2018,run2_DT_2018, run2_SiPixel_2018, 
-run2_HLTconditions_2018, run2_muon_2018, run2_egamma_2018, ctpps_2018)
+Run2_2018 = cms.ModifierChain(Run2_2017.copyAndExclude([run2_HEPlan1_2017, run2_muon_2017, run2_L1prefiring, run2_HLTconditions_2017, run2_egamma_2017, ctpps_2017]), run2_CSC_2018, run2_HCAL_2018, run2_HB_2018, run2_HE_2018,run2_DT_2018, run2_SiPixel_2018, run2_HLTconditions_2018, run2_muon_2018, run2_egamma_2018, ctpps_2018, run2_onlineMetaData_2018)


please keep the line readable

slava77 · 2019-08-08T15:45:08Z

After these changes all the MetaData/SCAL consumers can simply select the available Handle exploiting isValid method in order to preserve backward compatibility of the code.

I do not particularly like the strategy of letting the consumer code to pick one of the available data collections in the stream. This is OK in some exceptional cases. In most cases we know at the configuration stage what needs to be consumed.

cmsbuild · 2019-08-08T15:49:40Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-35450d/1904/summary.html

Comparison Summary:

No significant changes to the logs found
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 33
DQMHistoTests: Total histograms compared: 2715989
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2715659
DQMHistoTests: Total skipped: 329
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
Checked 137 log files, 14 edm output root files, 33 DQM output files

fabiocos

@arossi83 please find a few comments below, I think several comments suggest a possibly different approach than the one chosen here

fabiocos · 2019-08-17T14:32:38Z

Configuration/Eras/python/Era_Run2_2018_cff.py

-Run2_2018 = cms.ModifierChain(Run2_2017.copyAndExclude([run2_HEPlan1_2017, run2_muon_2017, run2_L1prefiring, run2_HLTconditions_2017, run2_egamma_2017, ctpps_2017]),
-run2_CSC_2018, run2_HCAL_2018, run2_HB_2018, run2_HE_2018,run2_DT_2018, run2_SiPixel_2018, 
-run2_HLTconditions_2018, run2_muon_2018, run2_egamma_2018, ctpps_2018)
+Run2_2018 = cms.ModifierChain(Run2_2017.copyAndExclude([run2_HEPlan1_2017, run2_muon_2017, run2_L1prefiring, run2_HLTconditions_2017, run2_egamma_2017, ctpps_2017]), run2_CSC_2018, run2_HCAL_2018, run2_HB_2018, run2_HE_2018,run2_DT_2018, run2_SiPixel_2018, run2_HLTconditions_2018, run2_muon_2018, run2_egamma_2018, ctpps_2018, run2_onlineMetaData_2018)


@arossi83 please split the statement among multiple lines as previously done

fabiocos · 2019-08-17T14:45:32Z

Configuration/StandardSequences/python/RawToDigi_cff.py

@@ -81,6 +80,12 @@
 muonRPCDigis.InputLabel = 'rawDataCollector'
 castorDigis.InputLabel = 'rawDataCollector'

+#Add OnlineMetaData only from Run2_2018
+from Configuration.Eras.Modifier_run2_onlineMetaData_2018_cff import run2_onlineMetaData_2018


summarizing the comments in this thread and during ORP meetings: the use case for which this development is thought, i.e. #27677, seems to be better served by customizing the consumer to use one or the other input, than the producer sequence as suggested here.

An approach similar to that suggested here is in #27638, but to me that is a quite limited use case for a standalone online application, while for a general use in production workflows the suggestion by @makortel and @slava77 make more sense.

I suggest to review the proposal along these lines.

Let me add that if the onlineMetaDataDigis producer doesn't work for data/MC <= 2017, then, I think, it might be useful to add it to the Task with a Modifier so that the producer is available only in scenarios where it makes sense. Does scalersRawToDigi work for data/MC >= 2018?

(but regardless of that I still think that it would be better for the client side (as well) to customize the client configuration with a Modifier to consume only the needed product)

fabiocos · 2019-08-18T20:15:45Z

hold

cmsbuild · 2019-08-18T20:16:12Z

Pull request has been put on hold by @fabiocos
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

fabiocos · 2019-09-10T08:02:09Z

@arossi83 is there any plan to review this proposal along the lines of #27676 (review) ?

arossi83 · 2019-09-10T10:23:16Z

@arossi83 is there any plan to review this proposal along the lines of #27676 (review) ?

@fabiocos yes, there is a plan. I'm just back from vacation and I will work on this

fabiocos · 2019-09-23T09:02:08Z

@arossi83 any news?

fabiocos · 2019-10-09T08:49:28Z

@arossi83 is there a plan to update this PR?

arossi83 · 2019-10-11T11:46:30Z

@arossi83 is there a plan to update this PR?

Hi @fabiocos, sorry I didn't have so much time to work on this. In any case since to solutions suggested implies a change on consumer side instead that on producer I suggest to close this PR. Any other action will affect completely different pieces of code. @mmusich do you agree?

fabiocos · 2019-10-30T09:02:22Z

@arossi83 @mmusich I will close this PR, waiting for a new proposal, or to reopen it in case a significant update is made

Select the unpacker to be run (between scalersRawToDigi and onlineMet…

f1ccf28

…aDataDigis) via Era modifier

cmsbuild added this to the CMSSW_11_0_X milestone Aug 2, 2019

cmsbuild added code-checks-pending comparison-pending operations-pending orp-pending pending-signatures tests-pending labels Aug 2, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 2, 2019

arossi83 mentioned this pull request Aug 2, 2019

Introduce changes in order to switch from LumiScalers to MetaData #27677

Closed

cmsbuild added tests-started and removed tests-pending labels Aug 5, 2019

cmsbuild added tests-rejected and removed tests-started labels Aug 5, 2019

cmsbuild mentioned this pull request Aug 5, 2019

Complete the implementation of RPC Digi Collection Merger started in PR #24613 #27672

Merged

cmsbuild added tests-started and removed tests-pending labels Aug 8, 2019

cmsbuild added tests-approved and removed tests-started labels Aug 8, 2019

slava77 reviewed Aug 8, 2019

View reviewed changes

cmsbuild added comparison-available and removed comparison-pending labels Aug 8, 2019

fabiocos mentioned this pull request Aug 9, 2019

[DQM] Migrated subsystem availability per lumi plot to softFED #27638

Merged

fabiocos reviewed Aug 17, 2019

View reviewed changes

cmsbuild added the hold label Aug 18, 2019

This was referenced Aug 30, 2019

HGCAL: 1st version of hadronic reconstruction #27819

Merged

Add energy regression and particle identification to RecoHGCal/TICL. #27634

Closed

cmsbuild mentioned this pull request Sep 12, 2019

Squash all Patatrack developments on top of CMSSW_11_3_0_pre5 #27983

Closed

cmsbuild mentioned this pull request Oct 17, 2019

NanoAODv6, config fixes for UL #28196

Merged

cmsbuild mentioned this pull request Oct 29, 2019

[HBHE] Update MahiDebugger and fix nP in NNLS #28193

Closed

fabiocos closed this Oct 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Select the unpacker to be run (between SCAL and MetaData) via Era modifier #27676

Select the unpacker to be run (between SCAL and MetaData) via Era modifier #27676

arossi83 commented Aug 2, 2019 •

edited

Loading

cmsbuild commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

arossi83 commented Aug 2, 2019

fioriNTU commented Aug 2, 2019

mmusich commented Aug 2, 2019 •

edited

Loading

fabiocos commented Aug 5, 2019

fabiocos commented Aug 5, 2019

cmsbuild commented Aug 5, 2019 •

edited

Loading

mmusich commented Aug 5, 2019

makortel commented Aug 5, 2019

cmsbuild commented Aug 5, 2019

cmsbuild commented Aug 5, 2019

cmsbuild commented Aug 5, 2019

cmsbuild commented Aug 8, 2019 •

edited

Loading

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

slava77 Aug 8, 2019

slava77 commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

fabiocos left a comment

fabiocos Aug 17, 2019

fabiocos Aug 17, 2019

makortel Aug 19, 2019

fabiocos commented Aug 18, 2019

cmsbuild commented Aug 18, 2019

fabiocos commented Sep 10, 2019

arossi83 commented Sep 10, 2019 •

edited

Loading

fabiocos commented Sep 23, 2019

fabiocos commented Oct 9, 2019

arossi83 commented Oct 11, 2019

fabiocos commented Oct 30, 2019

Select the unpacker to be run (between SCAL and MetaData) via Era modifier #27676

Select the unpacker to be run (between SCAL and MetaData) via Era modifier #27676

Conversation

arossi83 commented Aug 2, 2019 • edited Loading

PR description:

PR validation:

if this PR is a backport please specify the original PR:

cmsbuild commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

arossi83 commented Aug 2, 2019

fioriNTU commented Aug 2, 2019

mmusich commented Aug 2, 2019 • edited Loading

fabiocos commented Aug 5, 2019

fabiocos commented Aug 5, 2019

cmsbuild commented Aug 5, 2019 • edited Loading

mmusich commented Aug 5, 2019

makortel commented Aug 5, 2019

cmsbuild commented Aug 5, 2019

cmsbuild commented Aug 5, 2019

cmsbuild commented Aug 5, 2019

cmsbuild commented Aug 8, 2019 • edited Loading

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

slava77 Aug 8, 2019

Choose a reason for hiding this comment

slava77 commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

fabiocos left a comment

Choose a reason for hiding this comment

fabiocos Aug 17, 2019

Choose a reason for hiding this comment

fabiocos Aug 17, 2019

Choose a reason for hiding this comment

makortel Aug 19, 2019

Choose a reason for hiding this comment

fabiocos commented Aug 18, 2019

cmsbuild commented Aug 18, 2019

fabiocos commented Sep 10, 2019

arossi83 commented Sep 10, 2019 • edited Loading

fabiocos commented Sep 23, 2019

fabiocos commented Oct 9, 2019

arossi83 commented Oct 11, 2019

fabiocos commented Oct 30, 2019

arossi83 commented Aug 2, 2019 •

edited

Loading

mmusich commented Aug 2, 2019 •

edited

Loading

cmsbuild commented Aug 5, 2019 •

edited

Loading

cmsbuild commented Aug 8, 2019 •

edited

Loading

arossi83 commented Sep 10, 2019 •

edited

Loading