Move Sequence to Task for RecoLocalCalo config files #27474

jeongeun · 2019-07-10T06:02:12Z

PR description: Move Sequence to Task for RecoLocalCalo's config files

For a cms.Sequence object that is built from modules or cms.Sequence objects introduce a cms.Task with an equivalent content and make this cms.Sequence object directly from the newly introduced cms.Task. More details about Task objects
Remove cms.Sequence objects from the Reconstruction configuration.
I replaced the sequence by instantiating it with a task.

PR validation:

Tested in CMSSW_11_0_X_2019-07-07-2300 release.
The PR passes the basic test procedure suggested in the CMSSW PR instructions.

cmsbuild · 2019-07-10T06:02:43Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-07-10T06:10:06Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27474/10802

This PR adds an extra 20KB to repository

cmsbuild · 2019-07-10T06:10:38Z

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

It involves the following packages:

RecoLocalCalo/Configuration
RecoLocalCalo/HGCalRecProducers
RecoParticleFlow/PFClusterProducer

@perrotta, @cmsbuild, @kpedro88, @slava77 can you please review it and eventually sign? Thanks.
@edjtscott, @vandreev11, @sethzenz, @kpedro88, @felicepantaleo, @cbernet, @mmarionncern, @rovere, @lgray, @cseez, @bachtis, @hatakeyamak, @seemasharmafnal, @pfs 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

perrotta · 2019-07-10T07:52:04Z

RecoLocalCalo/Configuration/python/hgcalLocalReco_cff.py

+                                       HGCalRecHit,
+                                       hgcalLayerClusters,
+                                       hgcalMultiClusters,
+                                       particleFlowRecHitHGCTask,


I wonder whether one couldn't simply add here particleFlowRecHitHGC from RecoParticleFlow.PFClusterProducer.particleFlowRecHitHGC_cfi, instead of the task which only contains that module.

@lgray @felicepantaleo @rovere : was there a reason for having originally a sequence defined with one single module in RecoParticleFlow/PFClusterProducer/python/particleFlowRecHitHGC_cff.py? (As a corollary: could that cff file be removed now when moving to unscheduled execution?)

I cannot comment for the past history, but I see not special reason.
As for the Task, we could bypass it but I'm not sure if there is any added value.

perrotta · 2019-07-10T07:52:20Z

please test

cmsbuild · 2019-07-10T07:52:49Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1389/console Started: 2019/07/10 09:53

perrotta · 2019-07-10T08:49:55Z

RecoLocalCalo/Configuration/python/RecoLocalCalo_Commissioning_cff.py

@@ -26,11 +26,13 @@
 #
 # sequence CaloLocalReco and CaloGlobalReco
 #
-calolocalreco = cms.Sequence(ecalLocalRecoSequence+hcalLocalRecoSequence)
+calolocalrecoTask = cms.Task(ecalLocalRecoTask,hcalLocalRecoTask)
+calolocalreco = cms.Sequence(calolocalrecoTask)
 caloglobalreco = cms.Sequence(hcalGlobalRecoSequence)


Why not also migrating this sequence?

Thank you for your every comments.
I kept this sequence that have one module(hbhereco).
In this PR, I modified the sequences that has more than one modules.

In cmssw/RecoLocalCalo/Configuration/python/hcalGlobalReco_cff.py
from RecoLocalCalo.HcalRecProducers.HBHEIsolatedNoiseReflagger_cfi import *
hcalGlobalRecoSequence = cms.Sequence(hbhereco)

Indeed, this sequence is never used, it only includes hcalGlobalRecoSequence from RecoLocalCalo/Configuration/python/hcalGlobalReco_cff.py (which in its turn is only used for the globalreco_tracking reconstruction sequence) and it can be misleading here: I would suggest to just remove it

perrotta · 2019-07-10T08:52:23Z

RecoLocalCalo/Configuration/python/RecoLocalCalo_cff.py

@@ -20,7 +20,8 @@
 #
 # sequence CaloLocalReco and CaloGlobalReco
 #
-calolocalreco = cms.Sequence(ecalLocalRecoSequence+hcalLocalRecoSequence)
+calolocalrecoTask = cms.Task(ecalLocalRecoTask,hcalLocalRecoTask)
+calolocalreco = cms.Sequence(calolocalrecoTask)
 caloglobalreco = cms.Sequence(hcalGlobalRecoSequence)


Why not also migrating this sequence?

Indeed, this sequence is never used, it only includes hcalGlobalRecoSequence from RecoLocalCalo/Configuration/python/hcalGlobalReco_cff.py (which in its turn is only used for the globalreco_tracking reconstruction sequence) and it can be misleading here: I would suggest to just remove it

perrotta · 2019-07-10T09:18:57Z

please abort

cmsbuild · 2019-07-10T09:21:25Z

Jenkins tests are aborted.

perrotta · 2019-07-10T09:22:31Z

please test workflow 29093.52

cmsbuild · 2019-07-10T09:23:02Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1390/console Started: 2019/07/10 11:23

kpedro88 · 2019-07-13T01:36:44Z

+upgrade

civanch · 2019-07-13T16:53:51Z

+1

perrotta · 2019-07-14T11:56:17Z

For completeness, and for simmetry with the other reconstruction sequences, one could further adapt also Configuration/StandardSequences/python/ReconstructionHeavyIons_cff.py
On the other hand, this PR has now already got all other signatures: let postpone that update for when this PR will be merged

perrotta · 2019-07-14T11:57:21Z

+1

Migration from sequences to Tasks in local reco calo configs implemented as stated in the pr description
Jenkins tests pass and show no differences

cmsbuild · 2019-07-14T11:57:48Z

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

fabiocos · 2019-07-14T15:34:45Z

@jeongeun are you going to follow the suggestion of @perrotta in a separate PR?

perrotta · 2019-07-14T16:18:10Z

@jeongeun are you going to follow the suggestion of @perrotta in a separate PR?

@fabiocos , either she or myself we will, as soon as you are also ok with this PR and it will be merged

fabiocos · 2019-07-14T19:50:11Z

+1

@perrotta thanks

jeongeun added 3 commits July 8, 2019 13:43

Move Sequence to Task RecoLocalCalo configuration

0ce884a

Moving Sequence to Task RecoLocalCalo configuration

45e7790

fix error

eedb232

cmsbuild added this to the CMSSW_11_0_X milestone Jul 10, 2019

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending upgrade-pending labels Jul 10, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 10, 2019

perrotta reviewed Jul 10, 2019

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Jul 10, 2019

perrotta reviewed Jul 10, 2019

View reviewed changes

cmsbuild added tests-pending and removed tests-started labels Jul 10, 2019

cmsbuild added tests-started and removed tests-pending labels Jul 10, 2019

cmsbuild added the dqm-approved label Jul 12, 2019

cmsbuild added upgrade-approved and removed upgrade-pending labels Jul 13, 2019

cmsbuild added simulation-approved and removed simulation-pending labels Jul 13, 2019

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jul 14, 2019

cmsbuild added orp-approved and removed orp-pending labels Jul 14, 2019

cmsbuild merged commit 8211c3a into cms-sw:master Jul 14, 2019

cmsbuild mentioned this pull request Jul 14, 2019

DD4hep: Ecal Preshower Algorithm Migration #27518

Merged

This was referenced Jul 30, 2019

Moving sequence to task : RecoLocalMuon RecoEcal RecoEgamma RecoHI #27635

Closed

Moving Sequence to Task :RecoLocalMuon RecoMuon RecoEgamma RecoEcal RecoHI #27640

Merged

jeongeun mentioned this pull request Aug 28, 2019

Move Sequence to Task RecoJets RecoMET #27887

Merged

This was referenced Sep 18, 2019

Move Sequence to Task RecoParticleFlow RecoTracker RecoEgamma #28018

Closed

Move Sequence to Task for RecoParticleFlow RecoTracker RecoEgamma #28050

Merged

jeongeun mentioned this pull request Oct 29, 2019

Move Sequence to Task for RecoHI RecoEgamma #28300

Merged

jeongeun mentioned this pull request Nov 14, 2019

Move Sequence to Task for RecoTauTag RecoCTPPS #28399

Merged

jeongeun mentioned this pull request Dec 22, 2019

Move Sequence to Task Reconstruction_cff #28678

Merged

jeongeun deleted the MoveSeqToTask branch May 26, 2020 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Sequence to Task for RecoLocalCalo config files #27474

Move Sequence to Task for RecoLocalCalo config files #27474

jeongeun commented Jul 10, 2019

cmsbuild commented Jul 10, 2019

cmsbuild commented Jul 10, 2019

cmsbuild commented Jul 10, 2019

perrotta Jul 10, 2019

rovere Jul 10, 2019

perrotta commented Jul 10, 2019

cmsbuild commented Jul 10, 2019 •

edited

perrotta Jul 10, 2019

jeongeun Jul 10, 2019

perrotta Jul 11, 2019

perrotta Jul 10, 2019

perrotta Jul 11, 2019

perrotta commented Jul 10, 2019

cmsbuild commented Jul 10, 2019

perrotta commented Jul 10, 2019

cmsbuild commented Jul 10, 2019 •

edited

kpedro88 commented Jul 13, 2019

civanch commented Jul 13, 2019

perrotta commented Jul 14, 2019

perrotta commented Jul 14, 2019

cmsbuild commented Jul 14, 2019

fabiocos commented Jul 14, 2019

perrotta commented Jul 14, 2019

fabiocos commented Jul 14, 2019

Move Sequence to Task for RecoLocalCalo config files #27474

Move Sequence to Task for RecoLocalCalo config files #27474

Conversation

jeongeun commented Jul 10, 2019

PR description: Move Sequence to Task for RecoLocalCalo's config files

PR validation:

cmsbuild commented Jul 10, 2019

cmsbuild commented Jul 10, 2019

cmsbuild commented Jul 10, 2019

perrotta Jul 10, 2019

Choose a reason for hiding this comment

rovere Jul 10, 2019

Choose a reason for hiding this comment

perrotta commented Jul 10, 2019

cmsbuild commented Jul 10, 2019 • edited

perrotta Jul 10, 2019

Choose a reason for hiding this comment

jeongeun Jul 10, 2019

Choose a reason for hiding this comment

perrotta Jul 11, 2019

Choose a reason for hiding this comment

perrotta Jul 10, 2019

Choose a reason for hiding this comment

perrotta Jul 11, 2019

Choose a reason for hiding this comment

perrotta commented Jul 10, 2019

cmsbuild commented Jul 10, 2019

perrotta commented Jul 10, 2019

cmsbuild commented Jul 10, 2019 • edited

kpedro88 commented Jul 13, 2019

civanch commented Jul 13, 2019

perrotta commented Jul 14, 2019

perrotta commented Jul 14, 2019

cmsbuild commented Jul 14, 2019

fabiocos commented Jul 14, 2019

perrotta commented Jul 14, 2019

fabiocos commented Jul 14, 2019

cmsbuild commented Jul 10, 2019 •

edited

cmsbuild commented Jul 10, 2019 •

edited