Backport Puppi speedups [10_6_X] #37718

kpedro88 · 2022-04-27T22:04:36Z

PR description:

This PR is a combined backport of three speedups for Puppi:

Use reserve for vectors of candidates (see Puppi speedup: use reserve for vectors #37717)
Avoid creating/filling reco objects unnecessarily in useExistingWeights case (based on Simplify PUPPI Photon producer and change PUPPI isolation algo #27929)
Avoid use of unnecessary candidate vector and temporary object in useExistingWeights case (based on Fix a discrepancy in Puppi weights when running on MiniAOD w/ useExistingWeights=False #28033)

In my test of the useExistingWeights case (on a UL17 ttbar sample), the combination of these three changes reduced the CPU usage of PuppiProducer by 50%.

PR validation:

Code compiles and runs. CPU impact quantified with igprof. Compared AK8 jet pT in a ttbar workflow to confirm that no changes occurred.

if this PR is a backport please specify the original PR and why you need to backport that PR:

See above. Reason: to speed up ultra-legacy analysis processing.

…-sw#28033)

cmsbuild · 2022-04-27T22:05:04Z

A new Pull Request was created by @kpedro88 (Kevin Pedro) for CMSSW_10_6_X.

It involves the following packages:

CommonTools/PileupAlgos (reconstruction)

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

kpedro88 · 2022-04-27T22:07:48Z

please test

cmsbuild · 2022-04-28T01:03:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6021ad/24287/summary.html
COMMIT: 8bf214a
CMSSW: CMSSW_10_6_X_2022-04-24-0000/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37718/24287/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 3215686
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3215350
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: no differences found

jpata · 2022-04-29T12:41:54Z

type jetmet

clacaputo · 2022-05-02T10:13:21Z

test parameters

enable_test = profiling

clacaputo · 2022-05-02T10:13:34Z

@cmsbuild please test

smuzaffar · 2022-05-03T05:32:42Z

@jpata @kpedro88 , looks like the default workflow for profiling 35234.21 ( https://github.com/cms-sw/cms-bot/pull/1721/files ) is not available in 10.6.X that is why PR profiling job is failing . If profiling is needed for 10.6.X then can we adjust bot to use a different workflow?

jpata · 2022-05-03T11:12:21Z

Actually we were not sure what will happen when profiling in this backport, good to know that it fails due to the missing Phase2 workflow. I guess it's not critical here, one can rely on the profiling in the master.

kpedro88 · 2022-05-03T13:11:26Z

To my recollection, none of the prodlike workflows used for profiling are available in 10_6_X.

cmsbuild · 2022-05-03T14:15:58Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6021ad/24373/summary.html
COMMIT: 8bf214a
CMSSW: CMSSW_10_6_X_2022-05-01-0000/slc7_amd64_gcc700
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37718/24373/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 3215686
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3215350
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: no differences found

clacaputo · 2022-05-10T09:31:53Z

+reconstruction

backport of different speedup improvements for PUPPI
- WFs used in profiling aren't available for 10_6_X
- the effect of Puppi speedup: use reserve for vectors #37717 can be seen here Puppi speedup: use reserve for vectors #37717 (comment)
no reco changes expected or observed

cmsbuild · 2022-05-10T09:32:29Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_4_X is complete. 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)

perrotta

Since you are cleaning and you are not verbatim backporting the three PRs, you can also consider:

perrotta · 2022-05-11T12:58:00Z

CommonTools/PileupAlgos/plugins/PuppiProducer.cc

@@ -287,33 +282,36 @@ void PuppiProducer::produce(edm::Event& iEvent, const edm::EventSetup& iSetup) {
      const reco::PFCandidate *cand = dynamic_cast<const reco::PFCandidate*>(&aCand);
      pfCand.reset( new reco::PFCandidate( cand ? *cand : reco::PFCandidate(aCand.charge(), aCand.p4(), id) ) );
    }
-    LorentzVector pVec;

    //get an index to a pup in lCandidates: either fUseExistingWeights with no skips or get from fPuppiContainer
    int iPuppiMatched = fUseExistingWeights ? val : fPuppiContainer->recoToPup()[val];


if ( fUseExistingWeights ) { puppiP4s.emplace_back(lWeights[val]*aCand.px(), lWeights[val]*aCand.py(), lWeights[val]*aCand.pz(), lWeights[val]*aCand.energy()); } else { int iPuppiMatched = fPuppiContainer->recoToPup()[val]; if ( iPuppiMatched >= 0 ) {

kpedro88 · 2022-05-11T16:20:04Z

@perrotta done in the latest commit

cmsbuild · 2022-05-11T16:20:14Z

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

kpedro88 · 2022-05-11T16:21:29Z

please test

cmsbuild · 2022-05-11T19:53:18Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6021ad/24630/summary.html
COMMIT: 839d0c3
CMSSW: CMSSW_10_6_X_2022-05-08-0000/slc7_amd64_gcc700
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37718/24630/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: 35
DQMHistoTests: Total histograms compared: 3215686
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3215350
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: no differences found

clacaputo · 2022-05-12T13:39:35Z

+reconstruction

re-sign Backport Puppi speedups [10_6_X] #37718 (comment)

cmsbuild · 2022-05-12T13:39:58Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_4_X is complete. 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)

perrotta · 2022-05-12T13:58:27Z

+1

Code speedup, no changes on outputs

kpedro88 added 3 commits April 27, 2022 13:43

use reserve for vectors

1527607

backport 0eae59c: attempt to speed up useExistingWeights

b7ea30e

avoid use of lCandidates for useExistingWeights case (inspired by cms…

8bf214a

…-sw#28033)

cmsbuild added this to the CMSSW_10_6_X milestone Apr 27, 2022

cmsbuild added orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 27, 2022

cmsbuild added tests-started and removed tests-pending labels Apr 27, 2022

kpedro88 mentioned this pull request Apr 28, 2022

More speed improvements TreeMaker/TreeMaker#631

Merged

cmsbuild added tests-approved and removed tests-started labels Apr 28, 2022

cmsbuild added the jetmet label Apr 29, 2022

cmsbuild added tests-started and removed tests-approved labels May 2, 2022

cmsbuild added tests-approved and removed tests-started labels May 3, 2022

cmsbuild removed the reconstruction-pending label May 10, 2022

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures labels May 10, 2022

perrotta reviewed May 11, 2022

View reviewed changes

avoid extra conditional

839d0c3

cmsbuild added pending-signatures reconstruction-pending tests-pending and removed reconstruction-approved fully-signed tests-approved labels May 11, 2022

cmsbuild added tests-started and removed tests-pending labels May 11, 2022

cmsbuild added tests-approved and removed tests-started labels May 11, 2022

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels May 12, 2022

cmsbuild added orp-approved and removed orp-pending labels May 12, 2022

cmsbuild merged commit 6c0fc64 into cms-sw:CMSSW_10_6_X May 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backport Puppi speedups [10_6_X] #37718

Backport Puppi speedups [10_6_X] #37718

kpedro88 commented Apr 27, 2022

cmsbuild commented Apr 27, 2022

kpedro88 commented Apr 27, 2022

cmsbuild commented Apr 28, 2022

jpata commented Apr 29, 2022

clacaputo commented May 2, 2022

clacaputo commented May 2, 2022

smuzaffar commented May 3, 2022 •

edited

jpata commented May 3, 2022

kpedro88 commented May 3, 2022

cmsbuild commented May 3, 2022

clacaputo commented May 10, 2022

cmsbuild commented May 10, 2022

perrotta left a comment •

edited

perrotta May 11, 2022

kpedro88 commented May 11, 2022

cmsbuild commented May 11, 2022

kpedro88 commented May 11, 2022

cmsbuild commented May 11, 2022

clacaputo commented May 12, 2022

cmsbuild commented May 12, 2022

perrotta commented May 12, 2022

Backport Puppi speedups [10_6_X] #37718

Backport Puppi speedups [10_6_X] #37718

Conversation

kpedro88 commented Apr 27, 2022

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Apr 27, 2022

kpedro88 commented Apr 27, 2022

cmsbuild commented Apr 28, 2022

Comparison Summary

jpata commented Apr 29, 2022

clacaputo commented May 2, 2022

clacaputo commented May 2, 2022

smuzaffar commented May 3, 2022 • edited

jpata commented May 3, 2022

kpedro88 commented May 3, 2022

cmsbuild commented May 3, 2022

Comparison Summary

clacaputo commented May 10, 2022

cmsbuild commented May 10, 2022

perrotta left a comment • edited

Choose a reason for hiding this comment

perrotta May 11, 2022

Choose a reason for hiding this comment

kpedro88 commented May 11, 2022

cmsbuild commented May 11, 2022

kpedro88 commented May 11, 2022

cmsbuild commented May 11, 2022

Comparison Summary

clacaputo commented May 12, 2022

cmsbuild commented May 12, 2022

perrotta commented May 12, 2022

smuzaffar commented May 3, 2022 •

edited

perrotta left a comment •

edited