Updating Puppi Photon to ignore photons with large eta and to add back candidates #15429

violatingcp · 2016-08-11T14:30:21Z

Adds missing photon candidates with an eta cut to remove reconstruction issues at eta 3. These two features are needed to keep good performance in Puppi MET. Also, the test is script is updated with the actual computation of Puppi MET that should be used provided the weights are stored correctly.

…k missing photon candidates. These two features are needed to keep good performance in Puppi MET. Also, the test is script is updated with the actual computation of Puppi MET that should be used provided the weights are stored correctly.

cmsbuild · 2016-08-11T14:30:45Z

A new Pull Request was created by @violatingcp (Philip Harris) for CMSSW_8_0_X.

It involves the following packages:

CommonTools/PileupAlgos

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ahinzmann, @jdolen, @rappoccio this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

slava77 · 2016-08-11T14:34:42Z

@violatingcp
please make a PR for 81X.
We will not review 80X until there is 81X version.

slava77 · 2016-08-13T00:44:41Z

@cmsbuild please test

@violatingcp please add a link to a presentation in a JME meeting in the PR description (edit the top block #15429 (comment))

cmsbuild · 2016-08-13T00:45:04Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/14516/console

cmsbuild · 2016-08-13T01:30:43Z

-1

Tested at: bd4dd2d

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15429/14516/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

RelVals:

When I ran the RelVals I found an error in the following worklfows:
1306.0 step3

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step3_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log

135.4 step3

runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step3_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log

136.731 step3

runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT+HARVESTDR2/step3_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT+HARVESTDR2.log

1330.0 step3

runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step3_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.log

50202.0 step3

runTheMatrix-results/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/step3_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50.log

25202.0 step3

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step3_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log

slava77 · 2016-08-13T01:36:55Z

The errors are related to this PR
e.g. in wf 136.731
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15429/14516/runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT+HARVESTDR2/step3_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT+HARVESTDR2.log

----- Begin Fatal Exception 13-Aug-2016 03:00:16 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=PuppiPhoton label='puppiForMET'
Exception Message:
MissingParameter: Parameter 'eta' not found.
----- End Fatal Exception -------------------------------------------------

cvuosalo · 2016-08-19T14:55:37Z

@violatingcp: When will the problems in this PR be addressed? When will the 81X version be created?

cmsbuild · 2016-08-20T20:13:24Z

Pull request #15429 was updated. @cmsbuild, @cvuosalo, @slava77, @montjj, @davidlange6 can you please check and sign again.

slava77 · 2016-08-20T23:37:16Z

@cmsbuild please test

cmsbuild · 2016-08-20T23:39:19Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/14615/console

cmsbuild · 2016-08-21T01:35:38Z

+1
Tested at: 072976e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15429/14615/summary.html

cmsbuild · 2016-09-08T14:40:34Z

Pull request #15429 was updated. @cmsbuild, @cvuosalo, @slava77, @monttj, @davidlange6 can you please check and sign again.

slava77 · 2016-09-08T16:16:17Z

@mariadalfonso
should we expect imminent changes here following
#15669 (comment)
?
Please clarify at your earliest.
Thank you.

cvuosalo · 2016-09-08T18:11:06Z

@cmsbuild please test

cmsbuild · 2016-09-08T18:11:25Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15045/console

cmsbuild · 2016-09-08T19:30:43Z

+1
Tested at: 0b8cf50
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15429/15045/summary.html

cmsbuild · 2016-09-08T20:19:25Z

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

slava77 · 2016-09-08T20:20:50Z

given no feedback here yet, it looks like we will default to pick #15772 and this PR will be dropped

mariadalfonso · 2016-09-08T20:24:09Z

@slava77 the idea is that we want to pursue still this PR, this will follow closely the plan discussed in #15669 (comment) (81X)
only as fall back we keep the #15772

slava77 · 2016-09-08T20:56:51Z

On 9/8/16 1:24 PM, mariadalfonso wrote:

@slava77 https://github.com/slava77 the idea is that we want to pursue
still this PR
only as fall back we keep the #15772
#15772

Do you want to pursue this PR unchanged?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15429 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbkucZ94SnvF60dXvltH058wWbYIEks5qoG7qgaJpZM4JiLi1.

mariadalfonso · 2016-09-08T21:00:57Z

@slava77 we have the update commit with the DR in the 80X already just before the both started the test 0b8cf50

same we have in the 81X
b92cdaa

what I'm missing ?

slava77 · 2016-09-08T21:03:24Z

On 9/8/16 2:00 PM, mariadalfonso wrote:

@slava77 https://github.com/slava77 we have the update commit with the
DR in the 80X already just before the both started the test 0b8cf50
0b8cf50

same we have in the 81X
b92cdaa
b92cdaa

what I'm missing ?

#15669 (comment)
you said "if the DR test is pass then 1) .. 2)... "
This looked like updates are coming

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15429 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbgFO9djEqxzpfO7ih98WgIwE4BW5ks5qoHeKgaJpZM4JiLi1.

violatingcp · 2016-09-08T22:30:49Z

After discussion with @mariadalfonso we have decided #15669 contains all the fixes we would like. The proposal of two additional steps would require a larger re-engineering and consequently we stay with the current #15429 for the 80X release and #15669 for the 81X provided the checks come back with the expected good performance.

slava77 · 2016-09-08T23:11:48Z

@cvuosalo
I ran tests using the same set up as earlier for #15772

... timing looks about the same (unclear if this is surprising or just a good thing)

6.64 ms/ev ->         6.11 ms/ev puppiForMET

Compared to results of #15772 (black) this PR shows even better MET performance

cvuosalo · 2016-09-09T02:50:33Z

+1

For #15429 0b8cf50

Provisional approval to allow inclusion in imminent 80X release build.
Puppi changes to fix problems with photon candidates at high eta and to better handle PU.

This PR is related to #15562, #15669, and #15772 .

The code changes are satisfactory. Jenkins tests results seem not be accessible at the moment, but comparisons were successful. Extended tests results described above show the desired changes with no serious problems. When Jenkins and other test results become available, they will be reported in this thread.

slava77 · 2016-09-09T07:52:11Z

unhold

merging or rereco PRs is now almost done.
The hold was to highlight that the PR was just for the rereco release.

cvuosalo · 2016-09-09T16:46:27Z

Jenkins test results show numerous small differences, all of them either expected or too tiny to be of concern. Two examples are shown below.

From workflow 1330.0_ZMM_13, the elimination of high-eta photons can be seen:

A plot from workflow 136.731_RunSinglePh2016B shows the increase in the photon Et fraction for MET:

cvuosalo · 2016-09-09T18:16:29Z

A test of workflow 25200.0_ZEE_13 with 500 events against baseline CMSSW_8_0_X_2016-08-30-1100 shows numerous differences, most of them small, that are expected for this PR. Some examples are shown below.

$all_mini_testpr15429vsorig_zeepuwf25200p0c_patjets_slimmedjetspuppi__reco_obj_hfhadronenergyfraction$

cvuosalo · 2016-09-09T18:33:49Z

More results from the Z->ee test described in the previous entry.

$neuthadfrac$
$hfhadfrac$
$chhadfrac$

cmsbuild added this to the Next CMSSW_8_0_X milestone Aug 11, 2016

cmsbuild added reconstruction-pending analysis-pending pending-signatures tests-pending orp-pending comparison-pending labels Aug 11, 2016

cmsbuild added tests-started and removed tests-pending labels Aug 13, 2016

cmsbuild added tests-rejected and removed tests-started labels Aug 13, 2016

fixed Slava's & Maria's questions

072976e

cmsbuild added tests-pending and removed tests-rejected labels Aug 20, 2016

cmsbuild added tests-started and removed tests-pending labels Aug 20, 2016

cmsbuild added tests-approved and removed tests-started labels Aug 21, 2016

cmsbuild added reconstruction-pending tests-pending labels Sep 8, 2016

cmsbuild added tests-started and removed tests-pending labels Sep 8, 2016

cmsbuild added tests-approved and removed tests-started labels Sep 8, 2016

cmsbuild added comparison-available and removed comparison-pending labels Sep 8, 2016

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Sep 9, 2016

cmsbuild removed the hold label Sep 9, 2016

davidlange6 merged commit 1e24b5e into cms-sw:CMSSW_8_0_X Sep 9, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating Puppi Photon to ignore photons with large eta and to add back candidates #15429

Updating Puppi Photon to ignore photons with large eta and to add back candidates #15429

violatingcp commented Aug 11, 2016

cmsbuild commented Aug 11, 2016

slava77 commented Aug 11, 2016

slava77 commented Aug 13, 2016

cmsbuild commented Aug 13, 2016 •

edited

cmsbuild commented Aug 13, 2016

slava77 commented Aug 13, 2016

cvuosalo commented Aug 19, 2016

cmsbuild commented Aug 20, 2016

slava77 commented Aug 20, 2016

cmsbuild commented Aug 20, 2016 •

edited

cmsbuild commented Aug 21, 2016

cmsbuild commented Sep 8, 2016

slava77 commented Sep 8, 2016

cvuosalo commented Sep 8, 2016

cmsbuild commented Sep 8, 2016 •

edited

cmsbuild commented Sep 8, 2016

cmsbuild commented Sep 8, 2016

slava77 commented Sep 8, 2016

mariadalfonso commented Sep 8, 2016 •

edited

slava77 commented Sep 8, 2016

mariadalfonso commented Sep 8, 2016

slava77 commented Sep 8, 2016

violatingcp commented Sep 8, 2016

slava77 commented Sep 8, 2016 •

edited

cvuosalo commented Sep 9, 2016

slava77 commented Sep 9, 2016

cvuosalo commented Sep 9, 2016

cvuosalo commented Sep 9, 2016

cvuosalo commented Sep 9, 2016

Updating Puppi Photon to ignore photons with large eta and to add back candidates #15429

Updating Puppi Photon to ignore photons with large eta and to add back candidates #15429

Conversation

violatingcp commented Aug 11, 2016

cmsbuild commented Aug 11, 2016

slava77 commented Aug 11, 2016

slava77 commented Aug 13, 2016

cmsbuild commented Aug 13, 2016 • edited

cmsbuild commented Aug 13, 2016

slava77 commented Aug 13, 2016

cvuosalo commented Aug 19, 2016

cmsbuild commented Aug 20, 2016

slava77 commented Aug 20, 2016

cmsbuild commented Aug 20, 2016 • edited

cmsbuild commented Aug 21, 2016

cmsbuild commented Sep 8, 2016

slava77 commented Sep 8, 2016

cvuosalo commented Sep 8, 2016

cmsbuild commented Sep 8, 2016 • edited

cmsbuild commented Sep 8, 2016

cmsbuild commented Sep 8, 2016

slava77 commented Sep 8, 2016

mariadalfonso commented Sep 8, 2016 • edited

slava77 commented Sep 8, 2016

mariadalfonso commented Sep 8, 2016

slava77 commented Sep 8, 2016

violatingcp commented Sep 8, 2016

slava77 commented Sep 8, 2016 • edited

cvuosalo commented Sep 9, 2016

slava77 commented Sep 9, 2016

cvuosalo commented Sep 9, 2016

cvuosalo commented Sep 9, 2016

cvuosalo commented Sep 9, 2016

cmsbuild commented Aug 13, 2016 •

edited

cmsbuild commented Aug 20, 2016 •

edited

cmsbuild commented Sep 8, 2016 •

edited

mariadalfonso commented Sep 8, 2016 •

edited

slava77 commented Sep 8, 2016 •

edited