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.

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

Fix the customization functions of genpart pruning in NanoGEN #32431

Merged
merged 1 commit into from Dec 12, 2020

Conversation

kdlong
Copy link
Contributor

@kdlong kdlong commented Dec 9, 2020

PR description:

Fix bugs in genparticle pruning functions in NanoGEN. Previously the Nano pruner wasn't actually propogated to the GenParticleTable, and the mini pruner somehow had the source messed up.

PR validation:

Confirmed that both the Mini-like pruner and NanoAOD-like pruner work on MiniAOD and gridpack input

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32431/20306

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2020

A new Pull Request was created by @kdlong (Kenneth Long) for master.

It involves the following packages:

PhysicsTools/NanoAOD

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

@mariadalfonso
Copy link
Contributor

please test workflow 546,547,548

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2020

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aa6bda/11467/summary.html
CMSSW: CMSSW_11_3_X_2020-12-08-2300/slc7_amd64_gcc900

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2020

Comparison results are now available
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aa6bda/11467/summary.html
CMSSW: CMSSW_11_3_X_2020-12-08-2300/slc7_amd64_gcc900

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

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-aa6bda/546.0_DYToLL_M-50_13TeV_pythia8+DYToLL_M-50_13TeV_pythia8+NANOGENFromGen
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-aa6bda/547.0_DYToll01234Jets_5f_LO_MLM_Madgraph_LHE_13TeV+DYToll01234Jets_5f_LO_MLM_Madgraph_LHE_13TeV+Hadronizer_TuneCP5_13TeV_MLM_5f_max4j_LHE_pythia8+NANOGENFromGen
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-aa6bda/548.0_TTbar_Pow_LHE_13TeV+TTbar_Pow_LHE_13TeV+Hadronizer_TuneCP5_13TeV_powhegEmissionVeto2p_pythia8+NANOGENFromGen

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2747014
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2746979
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 35 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 153 log files, 37 edm output root files, 36 DQM output files

@mariadalfonso
Copy link
Contributor

+xpog

update in nanoGEN

@santocch
Copy link

+1

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

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 4ff72d9 into cms-sw:master Dec 12, 2020
mseidel42 added a commit to mseidel42/cmssw that referenced this pull request May 5, 2021
@mariadalfonso
Copy link
Contributor

mariadalfonso commented May 10, 2021

@kdlong @mseidel42 do we need to backport this as well to 10_6 ?

@kdlong
Copy link
Contributor Author

kdlong commented May 10, 2021

@mariadalfonso yes I should have, Markus discovered this again as part of his studies. Can it be included in the backport being prepared by @mseidel42 ?

mseidel42 added a commit to mseidel42/cmssw that referenced this pull request May 12, 2021
mseidel42 added a commit to mseidel42/cmssw that referenced this pull request May 12, 2021
mseidel42 added a commit to mseidel42/cmssw that referenced this pull request May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants