drop type spec and move sequence to task in CommonTools #33201

jeongeun · 2021-03-17T09:13:51Z

PR description:

optimization of the python configurations :
In this PR, 25 files changed. (previous PRs for CommonTools is PR#32464 )

drop type specifications where the original parameter exists.

CommonTools/ParticleFlow/python/Isolation/ 5 files
CommonTools/PileupAlgos/python/ 1 file
move cms.Sequence() to cms.Task()

CommonTools/ParticleFlow/python/ 11 files
CommonTools/ParticleFlow/python/GeneratorTools/ 1 file
CommonTools/ParticleFlow/python/Isolation/ 7 files

PR validation:

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

cmsbuild · 2021-03-17T09:21:48Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33201/21626

This PR adds an extra 28KB to repository

cmsbuild · 2021-03-17T09:22:11Z

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

It involves the following packages:

CommonTools/ParticleFlow
CommonTools/PileupAlgos

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @ahinzmann, @hatakeyamak, @jdolen, @gkasieczka, @cbernet, @clelange 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

perrotta · 2021-03-17T09:42:47Z

please test

cmsbuild · 2021-03-17T12:18:40Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6fc13f/13567/summary.html
COMMIT: e2b9158
CMSSW: CMSSW_11_3_X_2021-03-16-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33201/13567/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: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2639881
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 6
DQMHistoTests: Total successes: 2639846
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -5.491 KiB( 36 files compared)
DQMHistoSizes: changed ( 1000.0,... ): -0.145 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 1000.0,... ): -0.145 KiB MessageLogger/Errors
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: found differences in 2 / 32 workflows

perrotta · 2021-03-17T15:10:48Z

CommonTools/ParticleFlow/python/pfTaus_cff.py

   pfTausDiscriminationByIsolation
 )
+pfTausBaseSequence = cms.Sequence(pfTausBaseTask)


This is not needed now, maybe it can be cleaned up

perrotta · 2021-03-17T15:11:08Z

CommonTools/ParticleFlow/python/pfTaus_cff.py

    pfTauTagInfoProducer
 )
+pfTausPreSequence = cms.Sequence(pfTausPreTask)


This is not needed now, maybe it can be cleaned up

perrotta · 2021-03-17T15:16:07Z

CommonTools/ParticleFlow/python/pfType1MET_cff.py


-pfCorMET = cms.Sequence( pfRawMET * pfType1METChain )
+pfCorMETTask = cms.Task( pfRawMET , pfType1METChain )


Suggested change

pfCorMETTask = cms.Task( pfRawMET , pfType1METChain )

pfCorMETTask = cms.Task( pfRawMET , pfType1METChainTask )

(or, even better, remove L13 and rename pfType1METChainTask as pfType1METChain in L12)

perrotta · 2021-03-17T15:26:51Z

CommonTools/ParticleFlow/python/Isolation/photonPFIsolationValuesPFBRECO_cff.py

    phPFIsoValuePU04PFIdPFBRECO
    )
+photonPFIsolationValuesPFBRECOSequence = cms.Sequence(photonPFIsolationValuesPFBRECOTask)


Not needed any more, it can be removed

perrotta · 2021-03-17T15:27:06Z

CommonTools/ParticleFlow/python/Isolation/photonPFIsolationDepositsPFBRECO_cff.py

    phPFIsoDepositPUPFBRECO
    )
+photonPFIsolationDepositsPFBRECOSequence = cms.Sequence(photonPFIsolationDepositsPFBRECOTask)


Not needed any more, it can be removed

perrotta · 2021-03-17T15:32:57Z

CommonTools/ParticleFlow/python/Isolation/pfPhotonIsolation_cff.py

    )
-
+pfPhotonIsolationSequence = cms.Sequence(pfPhotonIsolationTask)


If one wants to clean up, one could now remove the definitions of photonPFIsolationDepositsSequence and photonPFIsolationValuesSequence from

RecoParticleFlow/PFProducer/python/photonPFIsolationValues_cff.py

RecoParticleFlow/PFProducer/python/photonPFIsolationDeposits_cff.py

cmsbuild · 2021-03-19T03:58:32Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33201/21658

This PR adds an extra 32KB to repository

cmsbuild · 2021-03-19T03:58:55Z

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

perrotta · 2021-03-19T07:04:25Z

please test

cmsbuild · 2021-03-19T09:42:23Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6fc13f/13616/summary.html
COMMIT: 9832dd4
CMSSW: CMSSW_11_3_X_2021-03-18-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33201/13616/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

Run3-hcx294 Update HCAL code in material budget studies for DD4HEP #33199 @bsunanda: Run3-hcx294 Update HCAL code in material budget studies for DD4HEP
DAQ: input source update and new output module for tests (11_3_X) #33198 @smorovic: DAQ: input source update and new output module for tests (11_3_X)
treat a predicted state too far as invalid in CosmicMuonSmoother #33168 @slava77: treat a predicted state too far as invalid in CosmicMuonSmoother
MTD geometry/reconstruction: update tests, invert order of forward DetLayers #33181 @fabiocos: MTD geometry/reconstruction: update tests, invert order of forward DetLayers
Run3-gex63 Clean up the debug statements for ECAL algos #33195 @bsunanda: Run3-gex63 Clean up the debug statements for ECAL algos
replace explicit configuration of puppi with a reference from cfipython/, generated by PuppiProducer::fillDescriptions #33207 @slava77: replace explicit configuration of puppi with a reference from cfipython/, generated by PuppiProducer::fillDescriptions

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6fc13f/13616/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6fc13f/13616/git-merge-result

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 130 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2639935
DQMHistoTests: Total failures: 673
DQMHistoTests: Total nulls: 6
DQMHistoTests: Total successes: 2639234
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -5.495 KiB( 36 files compared)
DQMHistoSizes: changed ( 1000.0,... ): -0.145 KiB MessageLogger/Warnings
DQMHistoSizes: changed ( 1000.0,... ): -0.145 KiB MessageLogger/Errors
DQMHistoSizes: changed ( 312.0 ): -0.148 KiB MessageLogger/Warnings
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: found differences in 2 / 32 workflows

perrotta · 2021-03-19T10:29:25Z

Since the last commit only cleaned up from unused sequences, I take for good that there were no crashes (which confirms, if needed, that really those sequences weren't used anywhere), while I consider the previous comparison to certify that this PR doesn't originate any change in reco (or whatever) output. The comparisons in the current test are biased by the additional PRs run together, instead.

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6fc13f/13616/summary.html
COMMIT: 9832dd4
CMSSW: CMSSW_11_3_X_2021-03-18-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33201/13616/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

Run3-hcx294 Update HCAL code in material budget studies for DD4HEP #33199 @bsunanda: Run3-hcx294 Update HCAL code in material budget studies for DD4HEP

DAQ: input source update and new output module for tests (11_3_X) #33198 @smorovic: DAQ: input source update and new output module for tests (11_3_X)

treat a predicted state too far as invalid in CosmicMuonSmoother #33168 @slava77: treat a predicted state too far as invalid in CosmicMuonSmoother

MTD geometry/reconstruction: update tests, invert order of forward DetLayers #33181 @fabiocos: MTD geometry/reconstruction: update tests, invert order of forward DetLayers

Run3-gex63 Clean up the debug statements for ECAL algos #33195 @bsunanda: Run3-gex63 Clean up the debug statements for ECAL algos

replace explicit configuration of puppi with a reference from cfipython/, generated by PuppiProducer::fillDescriptions #33207 @slava77: replace explicit configuration of puppi with a reference from cfipython/, generated by PuppiProducer::fillDescriptions

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6fc13f/13616/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6fc13f/13616/git-merge-result

Comparison Summary

Summary:

No significant changes to the logs found

Reco comparison results: 130 differences found in the comparisons

DQMHistoTests: Total files compared: 37

DQMHistoTests: Total histograms compared: 2639935

DQMHistoTests: Total failures: 673

DQMHistoTests: Total nulls: 6

DQMHistoTests: Total successes: 2639234

DQMHistoTests: Total skipped: 22

DQMHistoTests: Total Missing objects: 0

DQMHistoSizes: Histogram memory added: -5.495 KiB( 36 files compared)

DQMHistoSizes: changed ( 1000.0,... ): -0.145 KiB MessageLogger/Warnings

DQMHistoSizes: changed ( 1000.0,... ): -0.145 KiB MessageLogger/Errors

DQMHistoSizes: changed ( 312.0 ): -0.148 KiB MessageLogger/Warnings

Checked 155 log files, 37 edm output root files, 37 DQM output files

TriggerResults: found differences in 2 / 32 workflows

perrotta · 2021-03-19T10:29:58Z

+1

Technical, following what declared in the title and PR description
Jenkins tests pass

cmsbuild · 2021-03-19T10:30:14Z

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

silviodonato · 2021-03-19T14:11:13Z

+1

jeongeun added 2 commits March 11, 2021 13:36

drop type specs in CommonTools

4b9326f

move sequence to task

e2b9158

cmsbuild added this to the CMSSW_11_3_X milestone Mar 17, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Mar 17, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 17, 2021

cmsbuild added tests-started and removed tests-pending labels Mar 17, 2021

cmsbuild added tests-approved and removed tests-started labels Mar 17, 2021

perrotta reviewed Mar 17, 2021

View reviewed changes

cmsbuild mentioned this pull request Mar 17, 2021

replace explicit configuration of puppi with a reference from cfipython/, generated by PuppiProducer::fillDescriptions #33207

Merged

cleaned up not used sequence

8d73184

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Mar 19, 2021

Merge branch 'master' into common112Xupdates

9832dd4

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 19, 2021

cmsbuild added tests-started and removed tests-pending labels Mar 19, 2021

cmsbuild added tests-approved and removed tests-started labels Mar 19, 2021

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Mar 19, 2021

cmsbuild added orp-approved and removed orp-pending labels Mar 19, 2021

cmsbuild merged commit 6cc5d9b into cms-sw:master Mar 19, 2021

This was referenced Mar 19, 2021

Bump of CLHEP::Hep3Vector ClassVersion needed for (GEANT4 10.7.1) #33231

Closed

Refactor ProductResolvers #33230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drop type spec and move sequence to task in CommonTools #33201

drop type spec and move sequence to task in CommonTools #33201

jeongeun commented Mar 17, 2021

cmsbuild commented Mar 17, 2021

cmsbuild commented Mar 17, 2021

perrotta commented Mar 17, 2021

cmsbuild commented Mar 17, 2021

perrotta Mar 17, 2021

perrotta Mar 17, 2021

perrotta Mar 17, 2021

perrotta Mar 17, 2021

perrotta Mar 17, 2021

perrotta Mar 17, 2021

cmsbuild commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

perrotta commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

perrotta commented Mar 19, 2021

Comparison Summary

perrotta commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

silviodonato commented Mar 19, 2021


		pfCorMET = cms.Sequence( pfRawMET * pfType1METChain )
		pfCorMETTask = cms.Task( pfRawMET , pfType1METChain )

		)

		pfPhotonIsolationSequence = cms.Sequence(pfPhotonIsolationTask)

drop type spec and move sequence to task in CommonTools #33201

drop type spec and move sequence to task in CommonTools #33201

Conversation

jeongeun commented Mar 17, 2021

PR description:

PR validation:

cmsbuild commented Mar 17, 2021

cmsbuild commented Mar 17, 2021

perrotta commented Mar 17, 2021

cmsbuild commented Mar 17, 2021

Comparison Summary

perrotta Mar 17, 2021

Choose a reason for hiding this comment

perrotta Mar 17, 2021

Choose a reason for hiding this comment

perrotta Mar 17, 2021

Choose a reason for hiding this comment

perrotta Mar 17, 2021

Choose a reason for hiding this comment

perrotta Mar 17, 2021

Choose a reason for hiding this comment

perrotta Mar 17, 2021

Choose a reason for hiding this comment

cmsbuild commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

perrotta commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

Comparison Summary

perrotta commented Mar 19, 2021

Comparison Summary

perrotta commented Mar 19, 2021

cmsbuild commented Mar 19, 2021

silviodonato commented Mar 19, 2021