migrate ootPhoton to use GEDPhotonProducer #19264

slava77 · 2017-06-15T21:51:42Z

Legacy PhotonProducer was not good enough for ootPhotons: important functionality (some of showerShapeVariables) was missing.
There was an attempt to update the legacy photon producer in #19060, but it effectively lead to a copy of GEDPhotonProducer.
This PR makes needed changes to make GEDPhotonProducer work for non-GED setup (no PFEGammaCandidates on inputs)

I checked on wf 136.761 and see that only showerShapeVariables_ show up which were zero in the baseline. Kinematics of the ootPhotons and other variables did not change. The chosen workflow is not the best though (only 4 OOT photons in 200 events).
From visual inspection (confirmed by @kmcdermo ) this PR should produce correct outputs.

slava77 · 2017-06-15T21:51:53Z

@cmsbuild please test

cmsbuild · 2017-06-15T21:52:04Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20622/console Started: 2017/06/15 23:52

cmsbuild · 2017-06-15T21:52:06Z

A new Pull Request was created by @slava77 (Slava Krutelyov) for master.

It involves the following packages:

RecoEgamma/EgammaPhotonProducers

@perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@Sam-Harper, @rafaellopesdesa, @lgray this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

cmsbuild · 2017-06-15T23:20:16Z

-1

Tested at: 2ae60bf

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

I found follow errors while testing this PR

Failed tests: RelVals AddOn

RelVals:

When I ran the RelVals I found an error in the following worklfows:
140.53 step2

runTheMatrix-results/140.53_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI/step2_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI.log

136.731 step1

DAS Error

AddOn:

I found errors in the following addon tests:

cmsDriver.py RelVal -s HLT:HIon,RAW2DIGI,L1Reco,RECO --data --scenario=HeavyIons -n 10 --conditions auto:run2_data_HIon --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2016,Run2_HI --magField 38T_PostLS1 --processName=HLTRECO --filein file:RelVal_Raw_HIon_DATA.root --fileout file:RelVal_Raw_HIon_DATA_HLT_RECO.root : FAILED - time: date Fri Jun 16 00:58:46 2017-date Fri Jun 16 00:47:14 2017 s - exit: 16640
cmsDriver.py RelVal -s HLT:HIon,RAW2DIGI,L1Reco,RECO --mc --scenario=HeavyIons -n 10 --conditions auto:run2_mc_HIon --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2016,Run2_HI --magField 38T_PostLS1 --processName=HLTRECO --filein file:RelVal_Raw_HIon_MC.root --fileout file:RelVal_Raw_HIon_MC_HLT_RECO.root : FAILED - time: date Fri Jun 16 01:07:07 2017-date Fri Jun 16 00:55:20 2017 s - exit: 16640

cmsbuild · 2017-06-15T23:20:18Z

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

kmcdermo · 2017-06-16T00:13:09Z

Checking the runTheMatrix 140.53 logs:

----- Begin Fatal Exception 16-Jun-2017 00:00:35 CEST-----------------------
An exception of category 'GEDPhotonProducer' occurred while
[0] Processing Event run: 182124 lumi: 40 event: 1893722 stream: 0
[1] Running path 'RECOoutput_step'
[2] Prefetching for module PoolOutputModule/'RECOoutput'
[3] Calling method for module GEDPhotonProducer/'ootPhotons'
Exception Message:
Error! Can't get the product primary Vertex Collection
----- End Fatal Exception -------------------------------------------------

and then the AddOn test for the first one that failed:
----- Begin Fatal Exception 16-Jun-2017 00:57:21 CEST-----------------------
An exception of category 'GEDPhotonProducer' occurred while
[0] Processing Event run: 263689 lumi: 5 event: 395501 stream: 2
[1] Running path 'reconstruction_step'
[2] Calling method for module GEDPhotonProducer/'ootPhotons'
Exception Message:
Error! Can't get the product primary Vertex Collection
----- End Fatal Exception -------------------------------------------------

It seems that the HeavyIon 2011 config needs work to pick up the primary vertices...

cmsbuild · 2017-06-16T01:02:46Z

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

slava77 · 2017-06-16T01:02:56Z

@cmsbuild please test

140.53 runs OK locally with the last update.

cmsbuild · 2017-06-16T01:03:00Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20625/console Started: 2017/06/16 04:17

davidlange6 · 2017-06-16T12:24:30Z

its "fixed" in the 1100 IB

…

On Jun 16, 2017, at 2:22 PM, Slava Krutelyov ***@***.***> wrote: @cmsbuild please test @smuzaffar @davidlange6 was the issue with 136.731 step1 DAS Error resolved already? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

slava77 · 2017-06-16T12:28:41Z

On 6/16/17 5:24 AM, David Lange wrote: its "fixed" in the 1100 IB

OK. So, this test attempt will still fail, since jenkins is still on -2300

…

> On Jun 16, 2017, at 2:22 PM, Slava Krutelyov ***@***.***> wrote: > > @cmsbuild please test > > @smuzaffar @davidlange6 > was the issue with 136.731 step1 DAS Error resolved already? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub, or mute the thread. > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19264 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbta4GP_aruvB5DyIPtXHFdYzQ0tPks5sEnP-gaJpZM4N7yVd>.

slava77 · 2017-06-16T21:33:50Z

@cmsbuild please test

cmsbuild · 2017-06-16T21:34:06Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20658/console Started: 2017/06/16 23:35

cmsbuild · 2017-06-16T22:17:03Z

+1
Tested at: 08544d2
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19264/20658/summary.html

cmsbuild · 2017-06-16T22:17:06Z

Comparison job queued.

cmsbuild · 2017-06-16T23:29:19Z

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

Comparison Summary:

You potentially added 2 lines to the logs
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 22
DQMHistoTests: Total histograms compared: 1803136
DQMHistoTests: Total failures: 14920
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1788050
DQMHistoTests: Total skipped: 166
DQMHistoTests: Total Missing objects: 0
Checked 90 log files, 14 edm output root files, 22 DQM output files

slava77 · 2017-06-17T12:18:13Z

@kmcdermo @Sam-Harper
I would like to note the following

The comparisons show that there is a difference in caloPosition of the photon produced by GEDPhotonProducer compared to the PhotonProducer.
In the first case it's plain supercluster position
https://github.com/cms-sw/cmssw/blob/CMSSW_9_2_3/RecoEgamma/EgammaPhotonProducers/src/GEDPhotonProducer.cc#L542
In the PhotonProducer case it depends on r9 of the photon: high r9 photons are assigned a recalculated value based on SC seed hits only
https://github.com/cms-sw/cmssw/blob/CMSSW_9_2_3/RecoEgamma/EgammaPhotonProducers/src/PhotonProducer.cc#L376

Here is a sample of differences in workflow 136.761 after running on 400 events

* row    * instance * new.cPos.rho * old.cPos.rho * new.cPos.eta * old.cPos.eta * new.cPos.phi * old.cPos.phi * 
*       17 *        0 * 135.71141 * 135.76794 * 1.9394868 * 1.9376318 * 1.2528892 *  1.252316 *
*       26 *        0 * 136.14703 * 136.21090 * 2.9614338 * 2.9631238 * 0.9972192 * 0.9968107 *
*       48 *        0 * 139.29187 * 139.33931 * 1.9979250 * 1.9972713 * -0.355479 * -0.355387 *
*       49 *        0 * 136.83493 * 136.85549 * -3.017255 * -3.017694 * -1.057516 * -1.057394 *
*      347 *        0 * 137.80714 * 137.87281 * -1.110373 * -1.112018 * -0.778601 * -0.778133 *

I can guess that for a uniform treatment, you'd rather have the position the same as in gedPhotons, which use the same PF clustering as OOT photons.

Please confirm.

For the possible future migration of the legacy PhotonProducer to use the GEDPhotonProducer, this position recalculation will need to be added to GEDPhotonProducer.

kmcdermo · 2017-06-18T19:49:03Z

@slava77 , you are correct in asserting a more uniform treatment is preferred. Given that we use the same steps in forming superclusters for the ootPhotons as the gedPhotons, it would be better to have the same caloPosition assignment as the gedPhotons.

slava77 · 2017-06-19T16:02:04Z

+1

for #19264 08544d2

jenkins tests pass and comparisons with baseline show only a difference in reduced EB rechits, which corresponds to a small change in the definition of reference point to select these hits
local tests results are provided in the PR description

cmsbuild · 2017-06-19T16:02:26Z

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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

davidlange6 · 2017-06-19T19:23:02Z

+1

slava77devel added 2 commits June 15, 2017 20:19

allow running without pfEGammaCandidates on input

5b9b323

move ootPhotons to GEDPhotonProducer

2ae60bf

cmsbuild added this to the CMSSW_9_2_X milestone Jun 15, 2017

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jun 15, 2017

slava77 mentioned this pull request Jun 15, 2017

Add protection to zero size rechits in PhotonProducer, include more shower shape variables #19060

Closed

cmsbuild added tests-started and removed tests-pending labels Jun 15, 2017

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Jun 15, 2017

update inputs to use HI the new out-of-time photons

08544d2

cmsbuild added comparison-pending tests-pending and removed comparison-notrun tests-rejected labels Jun 16, 2017

cmsbuild removed the tests-pending label Jun 16, 2017

cmsbuild added the tests-started label Jun 16, 2017

cmsbuild added tests-pending and removed tests-started labels Jun 16, 2017

cmsbuild added tests-started and removed tests-pending labels Jun 16, 2017

cmsbuild added tests-approved and removed tests-started labels Jun 16, 2017

cmsbuild added comparison-available and removed comparison-pending labels Jun 16, 2017

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jun 19, 2017

cmsbuild added orp-approved and removed orp-pending labels Jun 19, 2017

cmsbuild merged commit d7c0a73 into cms-sw:master Jun 19, 2017

kmcdermo mentioned this pull request Jun 22, 2017

ootPhotons for 80X Legacy reprocessing #19404

Merged

slava77 mentioned this pull request Aug 8, 2017

Add MultiCluster derived electrons and photons #20045

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate ootPhoton to use GEDPhotonProducer #19264

migrate ootPhoton to use GEDPhotonProducer #19264

slava77 commented Jun 15, 2017

slava77 commented Jun 15, 2017

cmsbuild commented Jun 15, 2017 •

edited

cmsbuild commented Jun 15, 2017

cmsbuild commented Jun 15, 2017

cmsbuild commented Jun 15, 2017

kmcdermo commented Jun 16, 2017

cmsbuild commented Jun 16, 2017

slava77 commented Jun 16, 2017

cmsbuild commented Jun 16, 2017 •

edited

davidlange6 commented Jun 16, 2017 via email

slava77 commented Jun 16, 2017 via email

slava77 commented Jun 16, 2017

cmsbuild commented Jun 16, 2017 •

edited

cmsbuild commented Jun 16, 2017

cmsbuild commented Jun 16, 2017

cmsbuild commented Jun 16, 2017

slava77 commented Jun 17, 2017

kmcdermo commented Jun 18, 2017

slava77 commented Jun 19, 2017

cmsbuild commented Jun 19, 2017

davidlange6 commented Jun 19, 2017

migrate ootPhoton to use GEDPhotonProducer #19264

migrate ootPhoton to use GEDPhotonProducer #19264

Conversation

slava77 commented Jun 15, 2017

slava77 commented Jun 15, 2017

cmsbuild commented Jun 15, 2017 • edited

cmsbuild commented Jun 15, 2017

cmsbuild commented Jun 15, 2017

cmsbuild commented Jun 15, 2017

kmcdermo commented Jun 16, 2017

cmsbuild commented Jun 16, 2017

slava77 commented Jun 16, 2017

cmsbuild commented Jun 16, 2017 • edited

davidlange6 commented Jun 16, 2017 via email

slava77 commented Jun 16, 2017 via email

slava77 commented Jun 16, 2017

cmsbuild commented Jun 16, 2017 • edited

cmsbuild commented Jun 16, 2017

cmsbuild commented Jun 16, 2017

cmsbuild commented Jun 16, 2017

slava77 commented Jun 17, 2017

kmcdermo commented Jun 18, 2017

slava77 commented Jun 19, 2017

cmsbuild commented Jun 19, 2017

davidlange6 commented Jun 19, 2017

cmsbuild commented Jun 15, 2017 •

edited

cmsbuild commented Jun 16, 2017 •

edited

cmsbuild commented Jun 16, 2017 •

edited