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

Pps nano aod backport106 x #32616

Merged
merged 5 commits into from Jan 17, 2021

Conversation

jwill24
Copy link
Contributor

@jwill24 jwill24 commented Jan 7, 2021

PR description:

This is a backport for the PPS nanoAOD tables that were added to the 11_3_X branch (PR #31531).

The main changes are:

  • The naming scheme of the pps tracks "getter" functions in the ProtonProducer/ProtonFilter
  • Change from directory RecoPPS -> RecoCTPPS

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 7, 2021

A new Pull Request was created by @jwill24 (Justin Williams) for CMSSW_10_6_X.

It involves the following packages:

PhysicsTools/NanoAOD
RecoCTPPS/ProtonReconstruction

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

@perrotta
Copy link
Contributor

perrotta commented Jan 8, 2021

backport of #31531

@perrotta
Copy link
Contributor

perrotta commented Jan 8, 2021

@jwill24

@perrotta
Copy link
Contributor

perrotta commented Jan 8, 2021

PR description:

This is a backport for the PPS nanoAOD tables that were added to the 11_2_X branch (PR #31531).

@jwill24 : the PR #31531 was merged in 11_3_X, in fact. Only if you need it available also in 11_2_X, please prepare another backport for that release cycle

@jan-kaspar jan-kaspar mentioned this pull request Jan 8, 2021
@silviodonato
Copy link
Contributor

@jwill24 there is a conflict to be resolved. Probably another NanoAOD PR has been merged meanwhile

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 8, 2021

Pull request #32616 was updated. @perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso, @santocch can you please check and sign again.

@jwill24
Copy link
Contributor Author

jwill24 commented Jan 8, 2021

@jwill24 there is a conflict to be resolved. Probably another NanoAOD PR has been merged meanwhile

The conflicts have been resolved.

Copy link
Contributor

@mariadalfonso mariadalfonso left a comment

Choose a reason for hiding this comment

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

still missing additional fix from #32616 (comment)

PhysicsTools/NanoAOD/plugins/BuildFile.xml Outdated Show resolved Hide resolved
PhysicsTools/NanoAOD/python/protons_cff.py Outdated Show resolved Hide resolved
@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 8, 2021

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-75946a/12030/summary.html
COMMIT: a659687
CMSSW: CMSSW_10_6_X_2021-01-08-1100/slc7_amd64_gcc700

RelVals

1325.81_TTbar_13_106Xv1NanoAODINPUT+TTbar_13_106Xv1NanoAODINPUT+NANOEDMMC2017_106XMiniAODv1+HARVESTNANOAODMC2017_106XMiniAODv1/step2_TTbar_13_106Xv1NanoAODINPUT+TTbar_13_106Xv1NanoAODINPUT+NANOEDMMC2017_106XMiniAODv1+HARVESTNANOAODMC2017_106XMiniAODv1.log

----- Begin Fatal Exception 08-Jan-2021 19:34:21 CET-----------------------
An exception of category 'MultiplePlugins' occurred while
   [0] Constructing the EventProcessor
Exception Message:
The plugin 'SimpleXYZPointFlatTableProducer' is found in multiple files 
 '"pluginPhysicsToolsNanoAODPlugins.so"'
 '"pluginPhysicsToolsNanoAODPlugins.so"'
in directory '/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/32616/12030/CMSSW_10_6_X_2021-01-08-1100/lib/slc7_amd64_gcc700'.
The code must be changed so the plugin only appears in one plugin file. You will need to remove the macro which registers the plugin so it only appears in one of these files.
  If none of these files register such a plugin, then the problem originates in a library to which all these files link.
The plugin registration must be removed from that library since plugins are not allowed in regular libraries.
----- End Fatal Exception -------------------------------------------------
10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018+NanoFull_2018/step6_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018+NanoFull_2018.log

----- Begin Fatal Exception 08-Jan-2021 19:59:18 CET-----------------------
An exception of category 'MultiplePlugins' occurred while
   [0] Constructing the EventProcessor
Exception Message:
The plugin 'SimpleXYZPointFlatTableProducer' is found in multiple files 
 '"pluginPhysicsToolsNanoAODPlugins.so"'
 '"pluginPhysicsToolsNanoAODPlugins.so"'
in directory '/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/32616/12030/CMSSW_10_6_X_2021-01-08-1100/lib/slc7_amd64_gcc700'.
The code must be changed so the plugin only appears in one plugin file. You will need to remove the macro which registers the plugin so it only appears in one of these files.
  If none of these files register such a plugin, then the problem originates in a library to which all these files link.
The plugin registration must be removed from that library since plugins are not allowed in regular libraries.
----- End Fatal Exception -------------------------------------------------

@cmsbuild
Copy link
Contributor

Pull request #32616 was updated. @perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso, @santocch can you please check and sign again.

@perrotta
Copy link
Contributor

@jwill24

@jwill24
Copy link
Contributor Author

jwill24 commented Jan 12, 2021

@jwill24

@perrotta Done

@cmsbuild
Copy link
Contributor

Pull request #32616 was updated. @perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso, @santocch can you please check and sign again.

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-75946a/12239/summary.html
COMMIT: f9d5b9a
CMSSW: CMSSW_10_6_X_2021-01-12-1100/slc7_amd64_gcc700

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 3215502
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215167
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 6.388 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 1325.81,... ): 3.194 KiB Physics/NanoAODDQM
  • Checked 143 log files, 29 edm output root files, 35 DQM output files

1 similar comment
@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-75946a/12239/summary.html
COMMIT: f9d5b9a
CMSSW: CMSSW_10_6_X_2021-01-12-1100/slc7_amd64_gcc700

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 3215502
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215167
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 6.388 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 1325.81,... ): 3.194 KiB Physics/NanoAODDQM
  • Checked 143 log files, 29 edm output root files, 35 DQM output files

@perrotta
Copy link
Contributor

+1

  • The code for the filtered proton producer corresponds to the one merged in master (modulo different names for the PPS track getters which had been renamed between 10_6 and 11_3): this is fine with reco
  • Jenkins tests pass and show no differences in reco outputs wrt the baseline, as expected: the new plugin is only used for nanoAOD productions

@mariadalfonso
Copy link
Contributor

+xpog

DQM results and timing consistent with master

@cmsbuild
Copy link
Contributor

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_11_3_X is complete. 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)

@qliphy
Copy link
Contributor

qliphy commented Jan 17, 2021

+1

@cmsbuild cmsbuild merged commit dade8e9 into cms-sw:CMSSW_10_6_X Jan 17, 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

6 participants