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

Electron and Photon Regressions in RECO (76X) #11367

Merged

Conversation

lgray
Copy link
Contributor

@lgray lgray commented Sep 18, 2015

This PR implemented the latest regressions in RECO, they are no longer run in MiniAOD.

The Photon and Electron dataformats are extended to now save the extra set of commonly used shower shapes for ID and regression, saving space at the MiniAOD level since we are dropping strings.
full5x5 and fractionize shower shapes are saved.

Isolations are still embedded in MiniAOD. (it is WIP to put them directly in RECO)

Expected changes in electron and photon energies in AOD.
Validation plots posted below.

@ikrav VID functions in all respects in this PR now.

@bendavid @gpetruc

@lgray
Copy link
Contributor Author

lgray commented Sep 18, 2015

@cmsbuild please test

@cmsbuild cmsbuild added this to the Next CMSSW_7_6_X milestone Sep 18, 2015
@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_7_6_X.

Electron and Photon Regressions in RECO (76X)

It involves the following packages:

CommonTools/CandAlgos
DataFormats/EgammaCandidates
PhysicsTools/PatAlgos
RecoEgamma/EgammaElectronAlgos
RecoEgamma/EgammaElectronProducers
RecoEgamma/EgammaPhotonAlgos
RecoEgamma/EgammaPhotonProducers
RecoEgamma/EgammaTools
RecoEgamma/ElectronIdentification
RecoEgamma/PhotonIdentification
RecoHI/Configuration
RecoParticleFlow/Configuration

@cmsbuild, @cvuosalo, @vadler, @monttj, @slava77 can you please review it and eventually sign? Thanks.
@TaiSakuma, @rappoccio, @echapon, @mmarionncern, @imarches, @makortel, @acaudron, @dgulhan, @jdolen, @ferencek, @mandrenguyen, @yetkinyilmaz, @Sam-Harper, @nhanvtran, @schoef, @richard-cms, @MiheeJo, @jazzitup, @ahinzmann, @yenjie, @kurtejung, @gpetruc, @istaslis, @mariadalfonso, @pvmulder, @bachtis this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@lgray
Copy link
Contributor Author

lgray commented Sep 18, 2015

@richard-cms Does HI run the PF relinking after particleFlowTmp? I ask since the regressions are now applied in the finalized photon and electron producers that are run after PF, and did not see them being customized for HI.

@harmonicoscillator
Copy link
Contributor

Matt Nguyen is currently investigating running PF re-linking for HI. Our
initial tests showed that everything is working fine, but re-linking is not
actually enabled right now.

Alex

On Fri, Sep 18, 2015 at 1:38 PM, Lindsey Gray notifications@github.com
wrote:

@richard-cms https://github.com/richard-cms Does HI run the PF
relinking after particleFlowTmp? I ask since the regressions are now
applied in the finalized photon and electron producers that are run after
PF, and did not see them being customized for HI.


Reply to this email directly or view it on GitHub
#11367 (comment).

@harmonicoscillator
Copy link
Contributor

@mandrenguyen

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@lgray
Copy link
Contributor Author

lgray commented Sep 19, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

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

@lgray
Copy link
Contributor Author

lgray commented Sep 28, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@lgray
Copy link
Contributor Author

lgray commented Sep 29, 2015

Did one final check: for 2.25 TeV Z-primes the regressed resolution is better by 8%, the regressed scale is worse by 1%. This is acceptable for general use.

@slava77
Copy link
Contributor

slava77 commented Sep 29, 2015

there is now an improved ("pure") egamma performance as pfCandidates:

  • ele35 has higher peak and more centered mean (new and ref are flipped here: black is "new")
    all_origvssign594-pass-63708e9_singleelectron13pt35wf1317p0c_recopfcandidates_particleflow__reco_obj_pt19
  • single gamma10 electrons pfCandidates are also closer to 10 GeV (new and ref are flipped here: black is "new"):
    all_origvssign594-pass-63708e9_singlegamma13pt10wf1318p0c_recopfcandidates_particleflow__reco_obj_pt19
  • single gamma10 photons pfCandidates look slightly worse 10 GeV, I suppose this is just edge effects (new and ref are flipped here: black is "new"):
    all_origvssign594-pass-63708e9_singlegamma13pt10wf1318p0c_recopfcandidates_particleflow__reco_obj_pt39
  • gamma 35 PF candidate electrons are a bit better, look more symmetric around 35 (new and ref are flipped here: black is "new"):
    all_origvssign594-pass-63708e9_singlegamma13pt35wf1319p0c_recopfcandidates_particleflow__reco_obj_pt19
  • gamma 35 PF candidate photons are a better on the resolution side (new and ref are flipped here: black is "new"):
    all_origvssign594-pass-63708e9_singlegamma13pt35wf1319p0c_recopfcandidates_particleflow__reco_obj_pt39
  • ele 1000 are also slightly better as pfCandidates [manual fit gives mean pt/gen-1 move from -0.16% to -0.13%; sigma from 1.123% to 1.107%](new and ref are flipped here: black is):
    all_origvssign594-pass-63708e9_singleelectron13pt1000wf1316p0c_log10recopfcandidates_particleflow__reco_obj_pt20

Comparing to the previous iteration: the regression1Energy* in photons is now set to the values from regression2Energy*, which triggers changes in the plots.

Comparing with the original and new pfcandidates and egamma candidates: the consistency in momentum assignment is now restored.

@lgray
Copy link
Contributor Author

lgray commented Sep 29, 2015

@Slava7 10 GeV photon "worse" resolution is a convolution of edge effects + using all pf candidates (rather than those that are selected as real photons). This makes sense to me.

@slava77
Copy link
Contributor

slava77 commented Sep 29, 2015

+1

for #11367 63708e9

  • changes in the code are visually OK, implement what's declared in the PR:
  • tested locally in CMSSW_7_6_X_2015-09-22-1500
    • egamma candidates energies are updated (resolution improves), corresponding regression uncertainties change too (see plots from the first iteration)
    • pfCandidates corresponding to the egamma candidates change their energies consistent with the egamma candidate
    • new shower shape variables show up in the outputs as expected (no particular quality control of this was done from my side other than spot-checking the values are non-zero with some sensible distributions)
    • event size increases negligible at RECO and AOD level (under 0.5%); miniAOD size on 200 events goes up by 2% (this should decrease with more input events due to compression).
    • monitored physics variables change if related to the egamma candidate energies (reco::GsfElectron and reco::Photon GED collections) or come downstream of PF candidates
      • jets are not affected significantly for regular q/g jets
      • egamma candidates (reco::GsfElectron and reco::Photon GED collections) now have a better response (energy over gen) as expected
  • jenkins tests pass and comparisons with baseline are consistent with the more detailed tests

@slava77
Copy link
Contributor

slava77 commented Sep 29, 2015

@monttj please check this PR, the AT signature is needed.
Thank you.

@monttj
Copy link
Contributor

monttj commented Sep 30, 2015

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Sep 30, 2015
…gression_76X

Electron and Photon Regressions in RECO (76X)
@cmsbuild cmsbuild merged commit 4191dcc into cms-sw:CMSSW_7_6_X Sep 30, 2015
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

9 participants