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

NanoAODv6 updates, part 1 #28103

Merged
merged 15 commits into from Oct 6, 2019
Merged

Conversation

peruzzim
Copy link
Contributor

@peruzzim peruzzim commented Oct 2, 2019

PR description:

This pull request includes the bulk of NanoAOD updates for the upcoming V6 version.
Further minor tunings can be expected in a second pull request to follow.

  • Calculate (only if needed) and store DeepTauV2p1 in all releases
  • Add non-mass-decorrelated DeepDoubleX taggers
  • Store normalization separately for different parameter space points in one dataset (Randomized Parameters feature)
  • Fix definition of one quality bit in electron trigger object
  • Fix cosTheta vertex formula
  • Introduce run2_nanoAOD_106Xv1 modifier, meant to be used on 106X MiniAOD samples that do not have DeepTauV2p1 pre-calculated (e.g. UL2017)

Backports will follow to 10_2_X and 10_6_X.

PR validation:

This version was validated using the standard NanoAOD integration setup (running on MiniAOD created in 80X, 94X, 102X, 106X).

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28103/12103

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2019

A new Pull Request was created by @peruzzim for master.

It involves the following packages:

Configuration/Eras
Configuration/StandardSequences
PhysicsTools/NanoAOD

@kpedro88, @peruzzim, @cmsbuild, @franzoni, @fgolf, @santocch, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @gpetruc, @dgulhan this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@peruzzim
Copy link
Contributor Author

peruzzim commented Oct 2, 2019

please test workflow 1325.6,1325.7,1325.8,1329.1,136.7722,136.7952,136.8521

testing all NANOAOD workflows explicitly

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2019

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cfe0d6/2748/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 118 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2956830
  • DQMHistoTests: Total failures: 92
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2956397
  • DQMHistoTests: Total skipped: 341
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.56 KiB( 33 files compared)
  • DQMHistoSizes: changed ( 1325.7 ): 0.560 KiB Physics/NanoAODDQM
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@peruzzim
Copy link
Contributor Author

peruzzim commented Oct 3, 2019

+xpog

all as expected, changes in NANOAOD

@santocch
Copy link

santocch commented Oct 3, 2019

+1

@peruzzim
Copy link
Contributor Author

peruzzim commented Oct 4, 2019

Could you please review this? It would help speeding up backports to have it merged soon. Thanks!

@@ -0,0 +1,40 @@
#ifndef PhysicsTools_NanoAOD_EventStringOutputBranches_h
Copy link
Contributor

Choose a reason for hiding this comment

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

@peruzzim is there a special reason not to have this file integrated inside the .cc plugin implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it because I am including this header in PhysicsTools/NanoAOD/plugins/NanoAODOutputModule.cc.
If you have a suggestion on how this should be done differently, please let me know and I'll implement it in the next PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@peruzzim ok, you are correct, I agree

@fabiocos
Copy link
Contributor

fabiocos commented Oct 6, 2019

+operations

the update of Eras is coherent with the purpose and implementation of the PR

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 6, 2019

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

fabiocos commented Oct 6, 2019

+1

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