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

Add mini-isolation for leptons and a new collection of Isolated Tracks in MiniAOD #18548

Merged
merged 16 commits into from
May 23, 2017

Conversation

bjmarsh
Copy link
Contributor

@bjmarsh bjmarsh commented May 2, 2017

  • Store pre-computed mini-isolation variables in PAT Electrons and Muons. Stores the four components: charged hadron, neutral hadron, photon, and pileup.

  • Add a collection of new class pat::IsolatedTrack (inherits from reco::LeafCandidate). Picks out packed PF candidates that satisfy: charged, pT>5, (fromPV>1 || abs(dz)<0.1), (trackIso<5 GeV || relTrackIso<0.2 || relMiniTrackIso<0.2). Stores p4, pdgId, charge, dz, dxy, and the four components of both regular (dR=0.3) and mini isolation. Also a reference to the original packed candidate.

  • Tested on a sample of DY MC and validated the isolation values. In total, the additions add just over 100 bytes/event on average.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

A new Pull Request was created by @bjmarsh (Bennett Marsh) for master.

It involves the following packages:

DataFormats/PatCandidates
PhysicsTools/PatAlgos
PhysicsTools/PatUtils

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

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented May 2, 2017

-1

merge conflicts should be resolved first

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

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

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

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

@slava77
Copy link
Contributor

slava77 commented May 3, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 3, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19532/console Started: 2017/05/03 02:06

@cmsbuild
Copy link
Contributor

cmsbuild commented May 3, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

@slava77
Copy link
Contributor

slava77 commented May 15, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@@ -8,19 +8,18 @@
of packed PF candidates

Mini-Isolation reference: https://hypernews.cern.ch/HyperNews/CMS/get/susy/1991.html

typedef ROOT::Math::LorentzVector<ROOT::Math::PxPyPzE4D<double> > LorentzVector;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this typedef needed here inside the documentation lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not, that's a mistake.. must have accidentally pasted it after I deleted it below and didn't catch it. I can remove it

Copy link
Contributor

Choose a reason for hiding this comment

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

please remove.
Thank you.

@cmsbuild
Copy link
Contributor

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

@slava77
Copy link
Contributor

slava77 commented May 15, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 15, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19833/console Started: 2017/05/15 19:44

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 3366 differences found in the comparisons
  • DQMHistoTests: Total files compared: 24
  • DQMHistoTests: Total histograms compared: 1829394
  • DQMHistoTests: Total failures: 47951
  • DQMHistoTests: Total nulls: 39
  • DQMHistoTests: Total successes: 1781224
  • DQMHistoTests: Total skipped: 180
  • DQMHistoTests: Total Missing objects: 0
  • Checked 98 log files, 14 edm output root files, 24 DQM output files

@slava77
Copy link
Contributor

slava77 commented May 16, 2017

+1

for #18548 ca70584

here is a comparison in matrix wf 136.761 used for local tests of this PR previously red is for the latest version and black is with 7111208
all_sign906-pass-ca70584-patvssign906-pass-7111208-pat_runjetht2016ewf136p761c_patisolatedtracks_isolatedtracks__pat_obj_pfisolationdr03_photoniso

@slava77
Copy link
Contributor

slava77 commented May 19, 2017

@davidlange6
Please check this PR, it is in the way of integration of further developments.

Thank you.

@slava77
Copy link
Contributor

slava77 commented May 22, 2017

@davidlange6
Please check this PR, it is in the way of integration of further developments.

Thank you.

@davidlange6 davidlange6 merged commit c1ebfd3 into cms-sw:master May 23, 2017
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.

6 participants