Migrate module config in RecoTracker{IterativeTracking} to use default cfipython v3 #35194

jeongeun · 2021-09-08T11:15:51Z

PR description:

Optimization of the python configurations: Improve maintainability by cleaning up the duplicated and cloning from the default/reference configurations.

In this PR, 1 file changed.
RecoTracker/IterativeTracking/python/MixedTripletStep_cff.py
The previous PR is #34007.

PR validation:

Event Content comparison check was also done and there is no change with these updates.
Tested in CMSSW_12_1_X, the basic test all passed in the CMSSW PR instructions.

cmsbuild · 2021-09-08T11:23:09Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35194/25130

This PR adds an extra 16KB to repository

cmsbuild · 2021-09-08T11:23:36Z

A new Pull Request was created by @jeongeun (JeongEun Lee) for master.

It involves the following packages:

RecoTracker/IterativeTracking (reconstruction)

@jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @ebrondol, @gpetruc, @mmusich, @mtosi, @dgulhan 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

slava77 · 2021-09-09T15:03:09Z

@cmsbuild please test

cmsbuild · 2021-09-09T15:10:14Z

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9b49bf/18457/summary.html
COMMIT: fcec147
CMSSW: CMSSW_12_1_X_2021-09-08-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35194/18457/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation warning when building: See details on the summary page.

slava77 · 2021-09-09T16:03:32Z

RecoTracker/IterativeTracking/python/MixedTripletStep_cff.py

-mixedTripletStepSeedLayersA = cms.EDProducer('SeedingLayersEDProducer',
-     layerList = cms.vstring('BPix2+FPix1_pos+FPix2_pos', 'BPix2+FPix1_neg+FPix2_neg'),
-#    layerList = cms.vstring('BPix1+BPix2+BPix3', 
+from RecoTracker.TkSeedingLayers.seedingLayersEDProducer_cfi as _seedmod


Suggested change

from RecoTracker.TkSeedingLayers.seedingLayersEDProducer_cfi as _seedmod

import RecoTracker.TkSeedingLayers.seedingLayersEDProducer_cfi as _seedmod

@jeongeun please check/update this.

cmsbuild · 2021-09-16T02:29:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35194/25315

This PR adds an extra 20KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoTracker/IterativeTracking/python/MixedTripletStep_cff.py modified in PR(s): Move MSLayerKeeper* objects from thread-local into EventSetup #35269

cmsbuild · 2021-09-16T02:29:54Z

Pull request #35194 was updated. @jpata, @cmsbuild, @slava77 can you please check and sign again.

slava77 · 2021-09-16T13:42:04Z

@cmsbuild please test

slava77 · 2021-09-17T13:16:14Z

RecoTracker/IterativeTracking/python/MixedTripletStep_cff.py

-     layerList = cms.vstring('BPix2+FPix1_pos+FPix2_pos', 'BPix2+FPix1_neg+FPix2_neg'),
-#    layerList = cms.vstring('BPix1+BPix2+BPix3', 
+import RecoTracker.TkSeedingLayers.seedingLayersEDProducer_cfi as _seedmod
+mixedTripletStepSeedLayersA = _seedmod.seedingLayersEDProducer.clone(


there is now a redundant import RecoTracker.TkSeedingLayers.seedingLayersEDProducer_cfi as _mod on L164.
Please remove it there.

slava77 · 2021-09-17T13:17:49Z

RecoTracker/IterativeTracking/python/MixedTripletStep_cff.py

 #        'BPix1+BPix2+FPix1_pos', 'BPix1+BPix2+FPix1_neg', 
 #        'BPix1+FPix1_pos+FPix2_pos', 'BPix1+FPix1_neg+FPix2_neg', 
-#        'BPix2+FPix1_pos+FPix2_pos', 'BPix2+FPix1_neg+FPix2_neg'),
+#        'BPix2+FPix1_pos+FPix2_pos', 'BPix2+FPix1_neg+FPix2_neg'],
    BPix = cms.PSet(


the implementation in cfipython is

seedingLayersEDProducer = cms.EDProducer('SeedingLayersEDProducer', layerList = cms.vstring(), BPix = cms.PSet(), FPix = cms.PSet(), TIB = cms.PSet(), TID = cms.PSet(), TOB = cms.PSet(), TEC = cms.PSet(), MTIB = cms.PSet(), MTID = cms.PSet(), MTOB = cms.PSet(), MTEC = cms.PSet(),

So, I expect that the following should work (same for all other PSet in this module as well as in mixedTripletStepSeedLayersB in this file.

Suggested change

BPix = cms.PSet(

BPix = dict(

cmsbuild · 2021-09-23T05:28:42Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35194/25461

This PR adds an extra 20KB to repository

cmsbuild · 2021-09-23T05:29:06Z

Pull request #35194 was updated. @jpata, @cmsbuild, @slava77 can you please check and sign again.

slava77 · 2021-09-23T05:49:20Z

@cmsbuild please test

cmsbuild · 2021-09-23T09:39:02Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9b49bf/18850/summary.html
COMMIT: 5cd9719
CMSSW: CMSSW_12_1_X_2021-09-22-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35194/18850/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: 3211080
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3211058
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

slava77 · 2021-09-23T16:08:49Z

@jeongeun
thank you for the last update.
I noticed that similar patterns exist in other files.

git grep -16 seedingLayersEDProducer.clone shows some ideas

RecoTracker/ConversionSeedGenerators/python/ConversionStep2_cff.py
RecoTracker/ConversionSeedGenerators/python/ConversionStep_cff.py
RecoTracker/IterativeTracking/python/ElectronSeeds_cff.py
and so on

Please check and follow up in another PR.
Thank you.

slava77 · 2021-09-23T16:09:27Z

I noticed that similar patterns exist in other files.

I mean BPix = cms.PSet( vs BPix = dict(

slava77 · 2021-09-23T16:10:50Z

+reconstruction

for #35194 5cd9719

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no differences

cmsbuild · 2021-09-23T16:11:11Z

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

qliphy · 2021-09-24T00:55:10Z

+1

replace EDProducer with from cfipy and drop type spec

fcec147

cmsbuild added this to the CMSSW_12_1_X milestone Sep 8, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Sep 8, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 8, 2021

jeongeun mentioned this pull request Sep 8, 2021

Migrate module configuration in RecoVertex to use default cfipython v2 #35195

Merged

cmsbuild added tests-started and removed tests-pending labels Sep 9, 2021

cmsbuild added tests-rejected and removed tests-started labels Sep 9, 2021

slava77 reviewed Sep 9, 2021

View reviewed changes

cmsbuild mentioned this pull request Sep 14, 2021

Move MSLayerKeeper* objects from thread-local into EventSetup #35269

Merged

fix typo

78122f6

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Sep 16, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 16, 2021

slava77 reviewed Sep 17, 2021

View reviewed changes

Update MixedTripletStep_cff.py

5cd9719

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Sep 23, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 23, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 23, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 23, 2021

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Sep 23, 2021

cmsbuild added orp-approved and removed orp-pending labels Sep 24, 2021

cmsbuild merged commit cef44a4 into cms-sw:master Sep 24, 2021

This was referenced Oct 6, 2021

Migrate module config in RecoTracker{TkSeedingLayers} to use default seedingLayersEDProducer in cfipython #35549

Merged

Migrate module config in RecoTracker to use default seedingLayersEDProducer in cfipython v2 #35555

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate module config in RecoTracker{IterativeTracking} to use default cfipython v3 #35194

Migrate module config in RecoTracker{IterativeTracking} to use default cfipython v3 #35194

jeongeun commented Sep 8, 2021

cmsbuild commented Sep 8, 2021

cmsbuild commented Sep 8, 2021

slava77 commented Sep 9, 2021

cmsbuild commented Sep 9, 2021

slava77 Sep 9, 2021

slava77 Sep 13, 2021

cmsbuild commented Sep 16, 2021

cmsbuild commented Sep 16, 2021

slava77 commented Sep 16, 2021

slava77 Sep 17, 2021

slava77 Sep 17, 2021

cmsbuild commented Sep 23, 2021

cmsbuild commented Sep 23, 2021

slava77 commented Sep 23, 2021

cmsbuild commented Sep 23, 2021

slava77 commented Sep 23, 2021

slava77 commented Sep 23, 2021

slava77 commented Sep 23, 2021

cmsbuild commented Sep 23, 2021

qliphy commented Sep 24, 2021

	from RecoTracker.TkSeedingLayers.seedingLayersEDProducer_cfi as _seedmod
	import RecoTracker.TkSeedingLayers.seedingLayersEDProducer_cfi as _seedmod

Migrate module config in RecoTracker{IterativeTracking} to use default cfipython v3 #35194

Migrate module config in RecoTracker{IterativeTracking} to use default cfipython v3 #35194

Conversation

jeongeun commented Sep 8, 2021

PR description:

PR validation:

cmsbuild commented Sep 8, 2021

cmsbuild commented Sep 8, 2021

slava77 commented Sep 9, 2021

cmsbuild commented Sep 9, 2021

Build

slava77 Sep 9, 2021

Choose a reason for hiding this comment

slava77 Sep 13, 2021

Choose a reason for hiding this comment

cmsbuild commented Sep 16, 2021

cmsbuild commented Sep 16, 2021

slava77 commented Sep 16, 2021

slava77 Sep 17, 2021

Choose a reason for hiding this comment

slava77 Sep 17, 2021

Choose a reason for hiding this comment

cmsbuild commented Sep 23, 2021

cmsbuild commented Sep 23, 2021

slava77 commented Sep 23, 2021

cmsbuild commented Sep 23, 2021

Comparison Summary

slava77 commented Sep 23, 2021

slava77 commented Sep 23, 2021

slava77 commented Sep 23, 2021

cmsbuild commented Sep 23, 2021

qliphy commented Sep 24, 2021