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

Add nConstituents to fat jets. #32930

Merged
merged 3 commits into from May 23, 2021

Conversation

laurenhay
Copy link
Contributor

PR description:

For use with AK8 PF Candidate jet tables.

PR validation:

Trivial inspection. See jetTable.

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

#32927
Needed for NANO v9.

#### PR description:
For use with AK8 PF Candidate jet tables.

#### PR validation:
Trivial inspection. See jetTable.

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

NA
@smuzaffar
Copy link
Contributor

ping bot

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @laurenhay for CMSSW_10_6_X.

It involves the following packages:

PhysicsTools/NanoAOD

@cmsbuild, @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

@qliphy
Copy link
Contributor

qliphy commented Mar 2, 2021

@laurenhay Can you update this to the master PR #32927 accordingly?

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2021

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

@mariadalfonso
Copy link
Contributor

@laurenhay
to safeguard the current ongoing v8 production in 10_6_X, can you add something like
(run2_nanoAOD_106Xv1 and ~run2_nanoAOD_devel).toModify(fatJetTable , nConstituents = None)

@laurenhay
Copy link
Contributor Author

@laurenhay
to safeguard the current ongoing v8 production in 10_6_X, can you add something like
(run2_nanoAOD_106Xv1 and ~run2_nanoAOD_devel).toModify(fatJetTable , nConstituents = None)

Yes! Just pushed. Please let me know if this is what you were thinking.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2021

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

@mariadalfonso
Copy link
Contributor

@laurenhay
to safeguard the current ongoing v8 production in 10_6_X, can you add something like
(run2_nanoAOD_106Xv1 and ~run2_nanoAOD_devel).toModify(fatJetTable , nConstituents = None)

Yes! Just pushed. Please let me know if this is what you were thinking.

uhm, what you implemented switch off the addition for run2_nanoAOD_106Xv1 no matter how the run2_nanoAOD_devel is set.
what we want to wo switch off only when happens this (run2_nanoAOD_106Xv1 and ~run2_nanoAOD_devel)

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2021

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

@laurenhay
Copy link
Contributor Author

@laurenhay
to safeguard the current ongoing v8 production in 10_6_X, can you add something like
(run2_nanoAOD_106Xv1 and ~run2_nanoAOD_devel).toModify(fatJetTable , nConstituents = None)

Yes! Just pushed. Please let me know if this is what you were thinking.

uhm, what you implemented switch off the addition for run2_nanoAOD_106Xv1 no matter how the run2_nanoAOD_devel is set.
what we want to wo switch off only when happens this (run2_nanoAOD_106Xv1 and ~run2_nanoAOD_devel)

Sorry about that! I wasn't familiar with that convenient syntax! Hopefully I got it right now.

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 7, 2021

-1

Failed Tests: RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ce3140/13316/summary.html
COMMIT: 62cc888
CMSSW: CMSSW_10_6_X_2021-03-07-0000/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32930/13316/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

  • 1325.811325.81_TTbar_13_106Xv1NanoAODINPUT+TTbar_13_106Xv1NanoAODINPUT+NANOEDMMC2017_106XMiniAODv1+HARVESTNANOAODMC2017_106XMiniAODv1/step2_TTbar_13_106Xv1NanoAODINPUT+TTbar_13_106Xv1NanoAODINPUT+NANOEDMMC2017_106XMiniAODv1+HARVESTNANOAODMC2017_106XMiniAODv1.log

RelVals-INPUT

  • 136.8522136.8522_RunJetHT2018A_nanoUL+RunJetHT2018A_nanoUL+NANOEDM2018_106Xv1+HARVESTNANOAOD2018_106Xv1/step2_RunJetHT2018A_nanoUL+RunJetHT2018A_nanoUL+NANOEDM2018_106Xv1+HARVESTNANOAOD2018_106Xv1.log
  • 1325.611325.61_TTbar_13_106Xv1NanoAODINPUT+TTbar_13_106Xv1NanoAODINPUT+NANOAODMC2017_106XMiniAODv1/step2_TTbar_13_106Xv1NanoAODINPUT+TTbar_13_106Xv1NanoAODINPUT+NANOAODMC2017_106XMiniAODv1.log
  • 1325.811325.81_TTbar_13_106Xv1NanoAODINPUT+TTbar_13_106Xv1NanoAODINPUT+NANOEDMMC2017_106XMiniAODv1+HARVESTNANOAODMC2017_106XMiniAODv1/step2_TTbar_13_106Xv1NanoAODINPUT+TTbar_13_106Xv1NanoAODINPUT+NANOEDMMC2017_106XMiniAODv1+HARVESTNANOAODMC2017_106XMiniAODv1.log

@silviodonato
Copy link
Contributor

kind reminder @laurenhay

@gouskos
Copy link
Contributor

gouskos commented Mar 15, 2021

@laurenhay
please also include the modifiers for the DQM plots [not inlcude the additional plot in nanoAOD-v8].
You can use the following example as a guide: https://github.com/cms-sw/cmssw/pull/33075/files#diff-5f4338f7d7671c8348d9097ff44f255cc0b36d204c639782856a7c36cbc8fbda

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

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

@mariadalfonso
Copy link
Contributor

please test

Comment on lines 78 to 83
_sv_plots_nom = copy.deepcopy(nanoDQM.vplots.SV.plots)
_sv_plots_106Xv1 = cms.VPSet()
for plot in _sv_plots_nom:
if (plot.name.value() != "charge"):
_sv_plots_106Xv1.append(plot)
(run2_nanoAOD_106Xv1 & ~run2_nanoAOD_devel).toModify(nanoDQM.vplots.SV, plots = _sv_plots_106Xv1 )
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 this is still rebased on top of an old IB.
this part of the sv plots is unrelated to this PR and is already in the 10_6_X
https://github.com/cms-sw/cmssw/blob/CMSSW_10_6_X/PhysicsTools/NanoAOD/python/nanoDQM_cff.py#L105-L108

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry! Idk what happened since I did the rebasing in the CMSSW_10_6_X_2021-05-17-2300, but I've removed the sv lines.

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ce3140/15219/summary.html
COMMIT: 7624890
CMSSW: CMSSW_10_6_X_2021-05-20-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32930/15219/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: 36 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 3215551
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215216
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.183 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 136.8523 ): 0.183 KiB Physics/NanoAODDQM
  • Checked 143 log files, 29 edm output root files, 35 DQM output files
  • TriggerResults: no differences found

Get rid of extraneous sv no-change policy
@cmsbuild
Copy link
Contributor

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

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ce3140/15252/summary.html
COMMIT: b769e8e
CMSSW: CMSSW_10_6_X_2021-05-21-2300/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32930/15252/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: 36 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 3215551
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215216
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.183 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 136.8523 ): 0.183 KiB Physics/NanoAODDQM
  • Checked 143 log files, 29 edm output root files, 35 DQM output files
  • TriggerResults: no differences found

@mariadalfonso
Copy link
Contributor

+xpog

added variable to FatJet table; no changes for nanov8
full set of results here
https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/issues/87

@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_12_0_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 May 22, 2021

@cms-sw/xpog-l2 It seems there are some reco differences: "Reco comparison results: 36 differences found in the comparisons", and even on particleNetMD score and Nsubjettiness, do you know why?:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_10_6_X_2021-05-21-2300+ce3140/42922/validateJR/all_OldVSNew_RunJetHT2018CnanoEDM106Xv2wf136p8523/

@mariadalfonso
Copy link
Contributor

@cms-sw/xpog-l2 It seems there are some reco differences: "Reco comparison results: 36 differences found in the comparisons", and even on particleNetMD score and Nsubjettiness, do you know why?:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_10_6_X_2021-05-21-2300+ce3140/42922/validateJR/all_OldVSNew_RunJetHT2018CnanoEDM106Xv2wf136p8523/

the comparison you point is done based on a counter so when you add a variable, the new tree get offset respect to the reference.
This PR add a variable, so the amount of differences is not reliable; seen many other times
#31696 (comment)

if you look at the DQM results instead you see that the addition is only on the number of constituents of the FatJet (and for the nanov8 when there is no difference even there ) https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/issues/87

@qliphy
Copy link
Contributor

qliphy commented May 23, 2021

+1

@cmsbuild cmsbuild merged commit ad64f32 into cms-sw:CMSSW_10_6_X May 23, 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

8 participants