Adding parameters needed for PUjetID 94X and 102X #28827

alefisico · 2020-01-30T08:33:44Z

PR description:

Including the files and parameters needed for the latest PUjetID training for 94X and 102X samples.
Once merged, we need to backport this PR to the corresponding CMSSW release for the latest nanoAOD.
There is also a PR for the new weights for PUJetID Including weights for 94X and 102X PUjetID training cms-data/RecoJets-JetProducers#10 (comment)

PR validation:

Successfully run the runTheMatrix tests and here is the output: https://cernbox.cern.ch/index.php/s/XNTEPDN5VU01O84

Before submitting your pull requests, make sure you followed this checklist:

verify that the PR is really intended for the chosen branch
verify that changes follow CMS Naming, Coding, And Style Rules
verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

cmsbuild · 2020-01-30T08:34:08Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-01-30T08:38:54Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28827/13548

This PR adds an extra 16KB to repository

cmsbuild · 2020-01-30T08:39:13Z

A new Pull Request was created by @alefisico (Alejandro Gomez Espinosa) for master.

It involves the following packages:

RecoJets/JetProducers

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @jdolen, @ahinzmann, @jdamgov, @yslai, @nhanvtran, @gkasieczka, @clelange, @schoef, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

cmsbuild · 2020-01-30T10:24:27Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-01-30T10:37:14Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28827/13551

This PR adds an extra 16KB to repository

cmsbuild · 2020-01-30T10:37:32Z

Pull request #28827 was updated. @perrotta, @cmsbuild, @slava77 can you please check and sign again.

slava77 · 2020-01-30T13:20:06Z

@cmsbuild please test

cmsbuild · 2020-01-30T13:20:24Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4431/console Started: 2020/01/30 14:57

cmsbuild · 2020-01-30T15:14:51Z

+1
Tested at: 086826c
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-567459/4431/summary.html
CMSSW: CMSSW_11_1_X_2020-01-30-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-01-30T15:14:54Z

Comparison job queued.

cmsbuild · 2020-02-05T08:59:19Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-02-05T09:05:59Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28827/13636

This PR adds an extra 16KB to repository

cmsbuild · 2020-02-05T09:06:20Z

Pull request #28827 was updated. @perrotta, @cmsbuild, @slava77 can you please check and sign again.

perrotta · 2020-02-05T09:46:12Z

@cmsbuild please test

cmsbuild · 2020-02-05T09:46:35Z

The tests are being triggered in jenkins.
Tested with other pull request(s) cms-data/RecoJets-JetProducers#10
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4508/console Started: 2020/02/05 10:54

cmsbuild · 2020-02-05T11:33:04Z

+1
Tested at: 6af43a3
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-35ca6d/4508/summary.html
CMSSW: CMSSW_11_1_X_2020-02-04-2300
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-02-05T11:33:11Z

Comparison job queued.

cmsbuild · 2020-02-05T13:09:05Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 45 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2698043
DQMHistoTests: Total failures: 1261
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2696436
DQMHistoTests: Total skipped: 346
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

ahinzmann · 2020-02-05T14:16:16Z

@alefisico In these tests, the discriminator seems to always peak at -1, regardless of how much pileup is in the sample, e.g.
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_1_X_2020-02-04-2300+35ca6d/34963/validateJR/all_mini_OldVSNew_RunEGamma2018Cwf136p874/all_mini_OldVSNew_RunEGamma2018Cwf136p874c_patJets_slimmedJets__reRECO_obj___userFloats__3_.png

Was this also the case in the tests done in 10_2_X? It looks fishy to me.

camclean · 2020-02-07T08:55:58Z

Did the code change so that jets with pt > 50 GeV return -1?

perrotta · 2020-02-10T13:43:14Z

Is there any update on the explanation of the outputs?

slava77 · 2020-02-12T19:13:02Z

RecoJets/JetProducers/python/PileupJetIDCutParams_cfi.py

+    #4 Eta Categories  0-2.5 2.5-2.75 2.75-3.0 3.0-5.0
+
+    #Tight Id
+    Pt010_Tight    = cms.vdouble( 0.69, -0.35, -0.26, -0.21),


the values appear to be the same.
Do we really need to copy-paste this fairly large number of values?
Simply full_94x_chs_wp = full_102x_chs_wp.clone() would work.
Same comment applies to the other points.

These numbers are preliminary the same, but it will potentially change in the near future. (and could not be the same for all the years like now) Perhaps is better to include the structure now?

Looking back, indeed the values of the working points are rather different for different training cases.
What are the targets in training?

If the development strategy is to derive the working points from some fixed numerical target (like x% efficiency or rejection), then there would be very likely no accidental agreement. This, however doesn't seem to be the case this time.

If the WPs are weakly defined [this time?] with a replacement based on judgement calls,
it would be OK to copy-paste if the older values can update independently.

However it sounds like the development strategy this time is to first introduce placeholders to be replaced after some later processing.
[I'm judging from the history of the past updates and guessing from the symptoms of potentially broken WPs visible in the validation results].
If you feel like a shorter new_wp = old_wp.clone() #TEMPORARY is less acceptable,
please at least add an explicit comment inline that these are to be replaced soon.

slava77 · 2020-02-12T19:14:44Z

RecoJets/JetProducers/python/PileupJetIDParams_cfi.py

+                tmvaWeights   = cms.FileInPath("RecoJets/JetProducers/data/pileupJetId_102X_Eta2p5To2p75_chs_BDT.weights.xml.gz"),
+                tmvaVariables = cms.vstring(
+                    "nvtx"      ,
+                    "dR2Mean"   ,


it looks like the same variable name vector repeats for the first 3 ranges. These can be initialized from a separately defined (temporary) vector

True, I will change it

slava77 · 2020-02-12T19:17:07Z

RecoJets/JetProducers/python/PileupJetIDParams_cfi.py

+
+###################################################################################################################
+full_94x_chs = cms.PSet(
+        impactParTkThreshold = cms.double(1.),


is anything changing here other than the training file names and the working point PSet?
If not, a more compact and perhaps more maintainable way would be to use

full_94x_chs = full_102x_chs.clone(JetIdParams = full_94x_chs_wp) full_94x_chs.trainings[0].tmvaWeights = "RecoJets/JetProducers/data/pileupJetId_94X_Eta0p0To2p5_chs_BDT.weights.xml.gz" ...

Having the older IDs listed first may also be more maintainable for cases of future additions.

same as previous comment. We are currently using the same values for WP in these years, but they will potentially change in the near future. What is your suggestion?

full_94x_chs = full_102x_chs.clone(JetIdParams = full_94x_chs_wp) is applying different working points for 94X vs 102X. The suggestion in the earlier comment simply allows not to repeat what's known to be the same.

alefisico · 2020-02-18T08:32:55Z

Hi all,
we needed to change a couple of things after the bug that @ahinzmann found. Since now several things changed in CMSSW, I think it is better to close this PR and start clean.
New PR is #28984

adding new variables for PUjetID 94X and 102X

83bbdf9

cmsbuild added this to the CMSSW_11_1_X milestone Jan 30, 2020

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jan 30, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 30, 2020

fixing JetIDParams

086826c

cmsbuild added code-checks-pending and removed code-checks-approved labels Jan 30, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 30, 2020

nurfikri89 mentioned this pull request Jan 30, 2020

Update jet PU ID to 2017 version cms-nanoAOD/cmssw#492

Closed

cmsbuild added tests-started and removed tests-pending labels Jan 30, 2020

cmsbuild added tests-approved and removed tests-started labels Jan 30, 2020

cmsbuild added code-checks-pending comparison-pending tests-pending and removed requires-external tests-rejected labels Feb 5, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 5, 2020

cmsbuild added requires-external tests-started and removed tests-pending labels Feb 5, 2020

cmsbuild added tests-approved and removed tests-started labels Feb 5, 2020

cmsbuild added comparison-available and removed comparison-pending labels Feb 5, 2020

slava77 reviewed Feb 12, 2020

View reviewed changes

alefisico mentioned this pull request Feb 18, 2020

Adding parameters needed for PUjetID 94X and 102X v2 #28984

Merged

alefisico closed this Feb 18, 2020

singh-ramanpreet mentioned this pull request Dec 19, 2020

Pileup Id training for UL18 #32418

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding parameters needed for PUjetID 94X and 102X #28827

Adding parameters needed for PUjetID 94X and 102X #28827

alefisico commented Jan 30, 2020 •

edited

Loading

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

slava77 commented Jan 30, 2020

cmsbuild commented Jan 30, 2020 •

edited

Loading

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Feb 5, 2020

cmsbuild commented Feb 5, 2020

cmsbuild commented Feb 5, 2020

perrotta commented Feb 5, 2020

cmsbuild commented Feb 5, 2020 •

edited

Loading

cmsbuild commented Feb 5, 2020

cmsbuild commented Feb 5, 2020

cmsbuild commented Feb 5, 2020

ahinzmann commented Feb 5, 2020

camclean commented Feb 7, 2020

perrotta commented Feb 10, 2020

slava77 Feb 12, 2020

alefisico Feb 13, 2020

slava77 Feb 13, 2020

slava77 Feb 12, 2020

alefisico Feb 13, 2020

slava77 Feb 12, 2020

alefisico Feb 13, 2020

slava77 Feb 13, 2020

alefisico commented Feb 18, 2020

Adding parameters needed for PUjetID 94X and 102X #28827

Adding parameters needed for PUjetID 94X and 102X #28827

Conversation

alefisico commented Jan 30, 2020 • edited Loading

PR description:

PR validation:

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

slava77 commented Jan 30, 2020

cmsbuild commented Jan 30, 2020 • edited Loading

cmsbuild commented Jan 30, 2020

cmsbuild commented Jan 30, 2020

cmsbuild commented Feb 5, 2020

cmsbuild commented Feb 5, 2020

cmsbuild commented Feb 5, 2020

perrotta commented Feb 5, 2020

cmsbuild commented Feb 5, 2020 • edited Loading

cmsbuild commented Feb 5, 2020

cmsbuild commented Feb 5, 2020

cmsbuild commented Feb 5, 2020

ahinzmann commented Feb 5, 2020

camclean commented Feb 7, 2020

perrotta commented Feb 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alefisico commented Feb 18, 2020

alefisico commented Jan 30, 2020 •

edited

Loading

cmsbuild commented Jan 30, 2020 •

edited

Loading

cmsbuild commented Feb 5, 2020 •

edited

Loading