Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[NanoAOD] PileupJetId speed up #36848

Merged
merged 2 commits into from Feb 14, 2022
Merged

[NanoAOD] PileupJetId speed up #36848

merged 2 commits into from Feb 14, 2022

Conversation

nurfikri89
Copy link
Contributor

PR description:

This PR is to address the issue (cms-nanoAOD#586) regarding the slow speed of pileupJetId calculation in NanoAOD production. Now, only one instance of PileupJetIdProducer is scheduled and configured according to era. No changes in output is expected.

Below is a comparison of the cpu usage between CMSSW_12_3_0_pre4 and this PR using a TTJets ULMiniAODv2 sample as input with 5000 events.

Default (CMSSW_12_3_0_pre4)
circle_nanodefault

This PR
circle_nanomod_puidcleanup

PR validation:

passes the usual runTheMatrix test: runTheMatrix.py -l limited -i all --ibeos

Backports:

This PR should be backported 10_6_X if there is a plan for Ultra-Legacy NanoAODv10 production in the future.

@cmsbuild cmsbuild added this to the CMSSW_12_3_X milestone Feb 1, 2022
@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 1, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36848/28074

  • This PR adds an extra 20KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 1, 2022

A new Pull Request was created by @nurfikri89 (Nurfikri Norjoharuddeen) for master.

It involves the following packages:

  • PhysicsTools/NanoAOD (xpog)

@cmsbuild, @mariadalfonso, @gouskos, @fgolf can you please review it and eventually sign? Thanks.
@gpetruc, @swertz 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

@mariadalfonso
Copy link
Contributor

enable profiling

mariadalfonso
mariadalfonso reviewed Feb 1, 2022
View reviewed changes
PhysicsTools/NanoAOD/python/jets_cff.py Outdated
(run2_jme_2016 & ~tracker_apv_vfp30_2016 ).toModify( jetTable.variables.puId, doc="Pileup ID flags with 106X (2016) training")
(run2_jme_2016 & tracker_apv_vfp30_2016 ).toModify( jetTable.variables.puIdDisc, doc="Pileup ID discriminant with 106X (2016APV) training")
(run2_jme_2016 & tracker_apv_vfp30_2016 ).toModify( jetTable.variables.puId, doc="Pileup ID flags with 106X (2016APV) training")
run2_nanoAOD_102Xv1.toModify( jetTable.variables.puIdDisc, doc="Pileup ID discriminant with 102X (2018) training")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that changing order here is not good and lead to changes in the 102Xv1 and 2017 era.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will revert to the original ordering in the next commit.

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 1, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b10430/22121/summary.html
COMMIT: 0c26aa0
CMSSW: CMSSW_12_3_X_2022-01-31-2300/slc7_amd64_gcc10
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36848/22121/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: 43
  • DQMHistoTests: Total histograms compared: 3449612
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3449584
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
  • Checked 181 log files, 42 edm output root files, 43 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36848/28111

  • This PR adds an extra 20KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2022

Pull request #36848 was updated. @cmsbuild, @mariadalfonso, @gouskos, @fgolf can you please check and sign again.

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b10430/22153/summary.html
COMMIT: 27cd2ca
CMSSW: CMSSW_12_3_X_2022-02-02-1100/slc7_amd64_gcc10
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36848/22153/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

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

  • /pool/condor/dir_272580/jenkins/workspace/compare-root-files-short-matrix/data/PR-b10430/39434.0_TTbar_14TeV+2026D88+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal
  • /pool/condor/dir_272580/jenkins/workspace/compare-root-files-short-matrix/data/PR-b10430/39496.0_CloseByPGun_CE_E_Front_120um+2026D88+CE_E_Front_120um_GenSimHLBeamSpotHGCALCloseBy+DigiTrigger+RecoGlobal+HARVESTGlobal
  • /pool/condor/dir_272580/jenkins/workspace/compare-root-files-short-matrix/data/PR-b10430/39500.0_CloseByPGun_CE_H_Coarse_Scint+2026D88+CE_H_Coarse_Scint_GenSimHLBeamSpotHGCALCloseBy+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 46
  • DQMHistoTests: Total histograms compared: 3765022
  • DQMHistoTests: Total failures: 14
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3764986
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 45 files compared)
  • Checked 193 log files, 42 edm output root files, 46 DQM output files
  • TriggerResults: no differences found

@mariadalfonso
Copy link
Contributor

+xpog

re-arranged sequences with modifiers
no change in the output observed

@cmsbuild
Copy link
Contributor

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
Copy link
Contributor

qliphy commented Feb 14, 2022

+1

@cmsbuild cmsbuild merged commit f0e1ef2 into cms-sw:master Feb 14, 2022
@nurfikri89
Copy link
Contributor Author

@mariadalfonso Should this be backported?

@mariadalfonso
Copy link
Contributor

@mariadalfonso Should this be backported?

@nurfikri89 it's not needed for now, usually we do not backport speed up and next V10 Run2 nano production will be likely made with master

This was referenced Feb 14, 2022
Merged
Merged
Merged
Merged
Merged
@nurfikri89 nurfikri89 deleted the from1230pre4_CleanUpPileUpIDForNano branch July 15, 2022 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mariadalfonso mariadalfonso mariadalfonso left review comments

Assignees
No one assigned
Labels
code-checks-approved fully-signed orp-approved tests-approved xpog-approved
Projects
None yet
Milestone
CMSSW_12_3_X
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nurfikri89 @cmsbuild @mariadalfonso @qliphy