option to use deterministic event-based seed in SmearedJetProducer (for MT) #20240

kpedro88 · 2017-08-22T22:36:27Z

While upgrade my ntuple production code to use multithreading, I noticed the output jet collections differed for MC samples. I found the cause to be the use of random numbers in SmearedPATJetProducer. The solution is to use a deterministic seed based on the event number (thanks to @Dr15Jones for the suggestion). I followed the approach from https://github.com/cms-sw/cmssw/blob/CMSSW_9_3_0_pre4/RecoJets/JetProducers/plugins/VirtualJetProducer.cc#L272 (in a simplified manner). This resolved the regression I observed in my ntuple code when running with multiple threads.

This PR will be backported to 80X. Questions for jet people (e.g. @rappoccio, @blinkseb, @zdemirag):

Currently I have the useDeterministicSeed option disabled by default in the fillDescriptions(), so users would have to activate it manually. Should I enable it by default instead? This would change the default output. Edit: it was changed to be enabled by default.
Are other backports desired? (e.g. 92X)

…or MT)

cmsbuild · 2017-08-22T22:36:50Z

The code-checks are being triggered in jenkins.

cmsbuild · 2017-08-22T22:36:51Z

A new Pull Request was created by @kpedro88 (Kevin Pedro) for master.

It involves the following packages:

PhysicsTools/PatUtils

@perrotta, @cmsbuild, @monttj, @slava77 can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @imarches, @ahinzmann, @acaudron, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @gpetruc, @gkasieczka, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder, @seemasharmafnal, @JyothsnaKomaragiri this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

kpedro88 · 2017-08-22T22:36:59Z

please test

cmsbuild · 2017-08-22T22:37:42Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22435/console Started: 2017/08/23 01:12

cmsbuild · 2017-08-22T22:40:13Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20240/257

cmsbuild · 2017-08-23T00:45:35Z

+1
Tested at: 4514a35
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20240/22435/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
e739f38
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20240/22435/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20240/22435/git-merge-result

cmsbuild · 2017-08-23T00:45:37Z

Comparison job queued.

cmsbuild · 2017-08-23T02:22:41Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 26
DQMHistoTests: Total histograms compared: 2653934
DQMHistoTests: Total failures: 242
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2653503
DQMHistoTests: Total skipped: 189
DQMHistoTests: Total Missing objects: 0
Checked 107 log files, 14 edm output root files, 26 DQM output files

slava77 · 2017-08-24T14:57:37Z

@kpedro88
isn't this also needed in 92X?

kpedro88 · 2017-08-24T15:10:37Z

@slava77 wasn't sure, that's why I asked the jet experts in the PR description - I can just make a backport now if you think it's worthwhile

kpedro88 · 2017-08-24T15:12:09Z

Also, it was pointed out to me that this change solves a similar problem with reproducibility in CRAB if a job is split differently each time it's run, so maybe it should be enabled by default. I would still like to hear from jet experts on this (@rappoccio, @blinkseb, @zdemirag, ...).

kpedro88 · 2017-09-01T16:11:56Z

@slava77 is the latest commit okay? Let me know and then I'll update the backports

cmsbuild · 2017-09-01T16:11:57Z

The code-checks are being triggered in jenkins.

cmsbuild · 2017-09-01T16:11:57Z

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

cmsbuild · 2017-09-01T16:13:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20240/450

slava77 · 2017-09-01T17:15:56Z

@slava77 is the latest commit okay? Let me know and then I'll update the backports

yes, it covers the concerns mentioned so far.

slava77 · 2017-09-01T17:16:18Z

@cmsbuild please test

cmsbuild · 2017-09-01T17:16:36Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22656/console Started: 2017/09/01 19:17

cmsbuild · 2017-09-01T18:48:50Z

+1
Tested at: e4d33c5
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20240/22656/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
91fd772
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20240/22656/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20240/22656/git-merge-result

cmsbuild · 2017-09-01T18:48:53Z

Comparison job queued.

cmsbuild · 2017-09-01T20:26:26Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 26
DQMHistoTests: Total histograms compared: 2656222
DQMHistoTests: Total failures: 220
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2655813
DQMHistoTests: Total skipped: 189
DQMHistoTests: Total Missing objects: 0
Checked 107 log files, 14 edm output root files, 26 DQM output files

slava77 · 2017-09-05T21:12:33Z

PhysicsTools/PatUtils/python/patPFMETCorrections_cff.py

@@ -138,6 +138,7 @@
    variation = cms.int32(0),  # If not specified, default to 0

    seed = cms.uint32(37428479),  # If not specified, default to 37428479
+    useDeterministicSeed = cms.bool(True),


this does not match the PR description

Currently I have the useDeterministicSeed option disabled by default

I suppose, the description should be updated.

slava77 · 2017-09-05T21:36:36Z

+1

for #20240 e4d33c5

deterministic random numbers are implemented and enabled by default
jenkins tests pass and comparisons with baseline show some differences in MC workflows in METUnc_JetRes{Up,Down} plots as expected

kpedro88 · 2017-09-06T02:38:29Z

@slava77 updated PR description

davidlange6 · 2017-09-06T07:30:30Z

merge

option to use deterministic event-based seed in SmearedJetProducer (f…

4514a35

…or MT)

cmsbuild added this to the CMSSW_9_3_X milestone Aug 22, 2017

kpedro88 mentioned this pull request Aug 22, 2017

option to use deterministic event-based seed in SmearedJetProducer (for MT) [80X] #20241

Merged

cmsbuild added analysis-pending code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Aug 22, 2017

cmsbuild added tests-started and removed tests-pending labels Aug 22, 2017

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 22, 2017

cmsbuild added tests-approved and removed tests-started labels Aug 23, 2017

cmsbuild added comparison-available and removed comparison-pending labels Aug 23, 2017

kpedro88 mentioned this pull request Aug 24, 2017

option to use deterministic event-based seed in SmearedJetProducer (for MT) [92X] #20259

Merged

cmsbuild added comparison-pending tests-pending labels Sep 1, 2017

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 1, 2017

cmsbuild added tests-started and removed tests-pending labels Sep 1, 2017

cmsbuild added tests-approved and removed tests-started labels Sep 1, 2017

cmsbuild added comparison-available and removed comparison-pending labels Sep 1, 2017

slava77 reviewed Sep 5, 2017

View reviewed changes

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Sep 5, 2017

cmsbuild merged commit aa14a75 into cms-sw:master Sep 6, 2017

kpedro88 mentioned this pull request Sep 22, 2017

Updates to precision timing integration in reconstruction and miniaod #20338

Merged

kpedro88 mentioned this pull request Oct 24, 2017

make phase-2 track time "reco" (smearing) in TrackTimeValueMapProducer reproducible #20621

Closed

zdemirag mentioned this pull request Oct 8, 2018

JER Corrections cms-nanoAOD/cmssw#202

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

option to use deterministic event-based seed in SmearedJetProducer (for MT) #20240

option to use deterministic event-based seed in SmearedJetProducer (for MT) #20240

kpedro88 commented Aug 22, 2017 •

edited

Loading

cmsbuild commented Aug 22, 2017

cmsbuild commented Aug 22, 2017

kpedro88 commented Aug 22, 2017

cmsbuild commented Aug 22, 2017 •

edited

Loading

cmsbuild commented Aug 22, 2017

cmsbuild commented Aug 23, 2017

cmsbuild commented Aug 23, 2017

cmsbuild commented Aug 23, 2017

slava77 commented Aug 24, 2017

kpedro88 commented Aug 24, 2017

kpedro88 commented Aug 24, 2017

kpedro88 commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

slava77 commented Sep 1, 2017

slava77 commented Sep 1, 2017

cmsbuild commented Sep 1, 2017 •

edited

Loading

cmsbuild commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

slava77 Sep 5, 2017

slava77 commented Sep 5, 2017

kpedro88 commented Sep 6, 2017

davidlange6 commented Sep 6, 2017

option to use deterministic event-based seed in SmearedJetProducer (for MT) #20240

option to use deterministic event-based seed in SmearedJetProducer (for MT) #20240

Conversation

kpedro88 commented Aug 22, 2017 • edited Loading

cmsbuild commented Aug 22, 2017

cmsbuild commented Aug 22, 2017

kpedro88 commented Aug 22, 2017

cmsbuild commented Aug 22, 2017 • edited Loading

cmsbuild commented Aug 22, 2017

cmsbuild commented Aug 23, 2017

cmsbuild commented Aug 23, 2017

cmsbuild commented Aug 23, 2017

slava77 commented Aug 24, 2017

kpedro88 commented Aug 24, 2017

kpedro88 commented Aug 24, 2017

kpedro88 commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

slava77 commented Sep 1, 2017

slava77 commented Sep 1, 2017

cmsbuild commented Sep 1, 2017 • edited Loading

cmsbuild commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

cmsbuild commented Sep 1, 2017

slava77 Sep 5, 2017

Choose a reason for hiding this comment

slava77 commented Sep 5, 2017

kpedro88 commented Sep 6, 2017

davidlange6 commented Sep 6, 2017

kpedro88 commented Aug 22, 2017 •

edited

Loading

cmsbuild commented Aug 22, 2017 •

edited

Loading

cmsbuild commented Sep 1, 2017 •

edited

Loading