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

Puppi isolation photons miniAOD #15375

Merged

Conversation

ishvetso
Copy link
Contributor

@ishvetso ishvetso commented Aug 5, 2016

PUPPI isolation for slimmedPhotons. Isolation is computed by CITK module based on packedPFCanidates. PUPPI isolation is stored as userFloat "PUPPI_Isolation".

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

A new Pull Request was created by @ishvetso (Ivan Shvetsov) for CMSSW_8_1_X.

It involves the following packages:

PhysicsTools/PatAlgos

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @imarches, @ahinzmann, @acaudron, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @gpetruc, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder, @JyothsnaKomaragiri 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
Copy link
Contributor

slava77 commented Aug 5, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

-1

Tested at: 7d875ba

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15375/14378/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:
1330.0 step3

runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step3_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.log
136.731 step3
runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT+HARVESTDR2/step3_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT+HARVESTDR2.log
10024.0 step3
runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017.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

@ishvetso
Copy link
Contributor Author

ishvetso commented Aug 5, 2016

I see the error message and it doesn't find the value map that has to be
produced to get PUPPI isolation in the PATPhotonSlimmer

that means the module was probably not run

my understanding was that one need to keep the objet in the
PhysicsTools/PatAlgos/python/slimming/MicroEventContent_cff.py and it
would be added in Unscheduled mode

is this not the case?

Regards,

Ivan

05.08.16 16:32, cmsbuild пишет:

-1

Tested at: 7d875ba
7d875ba

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15375/14378/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:
1330.0 step3

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

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

10024.0 step3
runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017.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


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

Best regards,
Ivan Shvetsov.

@ishvetso
Copy link
Contributor Author

ishvetso commented Aug 5, 2016

should value maps for PUPPI photons isolation be added to prevalidationMiniAOD? Please clarify

modifyPhoton_(iConfig.getParameter<bool>("modifyPhotons")),
PUPPIIsolation_charged_hadrons_(consumes<edm::ValueMap<float> >(iConfig.getParameter<edm::InputTag>("PUPPIIsolationChargedHadrons"))),
PUPPIIsolation_neutral_hadrons_(consumes<edm::ValueMap<float> >(iConfig.getParameter<edm::InputTag>("PUPPIIsolationNeutralHadrons"))),
PUPPIIsolation_photons_(consumes<edm::ValueMap<float> >(iConfig.getParameter<edm::InputTag>("PUPPIIsolationPhotons")))
Copy link
Contributor

Choose a reason for hiding this comment

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

CMS style is to start variable names from lower case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

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

@slava77
Copy link
Contributor

slava77 commented Aug 5, 2016

@ishvetso are you done with the changes now?

@ishvetso
Copy link
Contributor Author

ishvetso commented Aug 5, 2016

yes, I think what you requested is done

@slava77
Copy link
Contributor

slava77 commented Aug 5, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2016

@ishvetso
Copy link
Contributor Author

ishvetso commented Aug 8, 2016

any further feedback?

@slava77
Copy link
Contributor

slava77 commented Aug 9, 2016

no feedback so far.
Probably later today.

@slava77
Copy link
Contributor

slava77 commented Aug 12, 2016

+1

for #15375 9b2503a

  • code changes are as described; affecting only miniAOD outputs (a new userFloat is saved in slimmedPhotons)
  • jenkins tests pass and comparisons with baseline show no difference
  • local tests on AODSIM as input (*) in 3200 events show that a new module now runs egmPhotonPUPPIIsolationForPhotons (which takes under 1ms per event) and that the size of the slimmedPhotons branch increases slightly (by 4 bytes per event, or 0.3% for this branch alone adding to 1.2 kB in the slimmedPhotons branch).

(*) /store/relval/CMSSW_8_1_0_pre9/RelValProdZEE_13_pmx25ns/AODSIM/PUpmx25ns_81X_mcRun2_asymptotic_v2-v1/10000/28ED6D8D-BD53-E611-B892-0025905AA9F0.root

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

4 participants