Muon puppi iso miniAOD 91x #18275

jshlee · 2017-04-09T13:32:54Z

adding muon puppi isolation into miniAOD
pat::Muons added puppi isolation

This is exactly the same as the implementation done for electrons, except

iso cone size is 04, since muon PFIso is 04
MuonPFIsolationWithConeVeto is slightly different to ElectronPFIsolationWithConeVeto (left this as is)
bugfix of PUPPINoLepton in PFIsolationSumProducerForPUPPI - before both PUPPINoLepton and PUPPI isolation was the same.

@calabria @kpedro88 @jhgoh

cmsbuild · 2017-04-09T13:33:24Z

A new Pull Request was created by @jshlee (Jason Lee) for master.

It involves the following packages:

DataFormats/PatCandidates
PhysicsTools/IsolationAlgos
PhysicsTools/PatAlgos
RecoMuon/MuonIsolation

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

cms-bot commands are listed here #13028

slava77 · 2017-04-09T18:01:07Z

@cmsbuild please test

@jshlee please add a link to slides (if available) which describe the changes together with some basic validation plots. Please also provide an estimate of file size increase in miniAOD.
Thank you

cmsbuild · 2017-04-09T18:01:25Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19036/console Started: 2017/04/09 20:03

cmsbuild · 2017-04-09T19:30:12Z

+1
Tested at: 96e6205
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18275/19036/summary.html

cmsbuild · 2017-04-09T19:30:14Z

Comparison job queued.

cmsbuild · 2017-04-09T21:04:49Z

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

Comparison Summary:

You potentially added 23 lines to the logs
Reco comparison results: 1640 differences found in the comparisons
DQMHistoTests: Total files compared: 23
DQMHistoTests: Total histograms compared: 1917243
DQMHistoTests: Total failures: 38745
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1878325
DQMHistoTests: Total skipped: 173
DQMHistoTests: Total Missing objects: 0
Checked 94 log files, 14 edm output root files, 23 DQM output files

jshlee · 2017-04-10T07:11:14Z

@slava77
Slides from Upgrade Performance Studies meeting https://indico.cern.ch/event/623426/contributions/2520808/attachments/1432129/2200377/muonReco_20170322.pdf

This adds 6 floats to pat::muons
File size comparision for 100 ttbar events (22834.0)

File step3_inMINIAODSIM.root Events 100
Branch Name | Average Uncompressed Size (Bytes/Event) | Average Compressed Size (Bytes/Event)
CMSSW_9_1_0_pre2 patMuons_slimmedMuons__RECO. 5218.32 3158.94
CMSSW_9_1_0_pre2+this PR patMuons_slimmedMuons__RECO. 5315.99 3234.31

slava77 · 2017-04-15T16:59:44Z

DataFormats/PatCandidates/interface/Muon.h

+      float puppiNoLeptonsNeutralHadronIso() const {return puppiNoLeptonsNeutralHadronIso_; }
+      float puppiNoLeptonsPhotonIso() const {return puppiNoLeptonsPhotonIso_; }
+      /// sets PUPPI isolations
+      void setIsolationPUPPI(float chargedhadrons_, float neutralhadrons_, float photons_)


trailing underscores in local scope variables are confusing: they are typically used for member data.
Please drop

slava77 · 2017-04-15T17:03:17Z

PhysicsTools/IsolationAlgos/plugins/CITKPFIsolationSumProducerForPUPPI.cc

-      else useValueMapForPUPPI = false;
+      else {
+	useValueMapForPUPPI = false;
+	usePUPPINoLepton = (c.exists("usePUPPINoLepton") ? c.getParameter<bool>("usePUPPINoLepton") : false);


fillDescriptions already defines the usePUPPINoLepton parameter and its default value (false).
Please change this to usePUPPINoLepton = c.getParameter<bool>("usePUPPINoLepton")

also, this appears to be a bugfix to the usePUPPINoLepton, which should explain why the electron puppi isolations have changed.

yep. electron puppi no lepton was incorrect.

slava77 · 2017-04-15T17:12:28Z

PhysicsTools/PatAlgos/plugins/PATMuonProducer.cc

@@ -244,6 +275,15 @@ void PATMuonProducer::produce(edm::Event & iEvent, const edm::EventSetup & iSetu
      if( embedPFCandidate_ ) aMuon.embedPFCandidate();
      fillMuon( aMuon, muonBaseRef, pfBaseRef, genMatches, deposits, isolationValues );

+      if (addPuppiIsolation_) {
+	aMuon.setIsolationPUPPI((*PUPPIIsolation_charged_hadrons)[muonBaseRef], (*PUPPIIsolation_neutral_hadrons)[muonBaseRef], (*PUPPIIsolation_photons)[muonBaseRef]);
+	aMuon.setIsolationPUPPINoLeptons((*PUPPINoLeptonsIsolation_charged_hadrons)[muonBaseRef], (*PUPPINoLeptonsIsolation_neutral_hadrons)[muonBaseRef], (*PUPPINoLeptonsIsolation_photons)[muonBaseRef]);


some line breaks would be nice to improve readability (or shorter local variable names)

slava77 · 2017-04-15T17:17:11Z

RecoMuon/MuonIsolation/python/muonIsolationPUPPI_cff.py

+        cms.PSet( isolationAlgo = cms.string('MuonPFIsolationWithConeVeto'),
+                  coneSize = cms.double(0.4),
+                  VetoThreshold = cms.double(0.0),
+                  VetoConeSize = cms.double(0.01),


it may be worth to add a comment that VetoConeSize corresponds to a square of the deltaR.

slava77 · 2017-04-17T19:05:39Z

jenkins reco comparisons show differences in slimmedElectron neutral hadron and photon isolation
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_9_1_X_2017-04-09-1100+18275/19295/validateJR/all_mini_OldVSNew_RunSinglePh2016Bwf136p731/
this is apparently from the change in https://github.com/cms-sw/cmssw/pull/18275/files#diff-e12c8057da6377bf2fb1be736a41576aR81

I think that this fix should be mentioned in the PR description and maybe also in the title.

removed _ from not member variables removed check of usePUPPINoLepton

jshlee · 2017-04-18T16:23:09Z

@slava77 - Thanks as always for your comments. I've updated the code with your suggestions.

Cheers,
Jason.

cmsbuild · 2017-04-18T16:23:23Z

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

slava77 · 2017-04-18T16:39:42Z

@cmsbuild please test

cmsbuild · 2017-04-18T16:39:58Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19221/console Started: 2017/04/18 18:51

cmsbuild · 2017-04-18T18:55:10Z

+1
Tested at: 7921cf9
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18275/19221/summary.html

cmsbuild · 2017-04-18T18:55:13Z

Comparison job queued.

cmsbuild · 2017-04-18T20:13:31Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 3294 differences found in the comparisons
DQMHistoTests: Total files compared: 23
DQMHistoTests: Total histograms compared: 1819510
DQMHistoTests: Total failures: 15623
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1803714
DQMHistoTests: Total skipped: 173
DQMHistoTests: Total Missing objects: 0
Checked 94 log files, 14 edm output root files, 23 DQM output files

slava77 · 2017-04-18T20:30:19Z

+1

for #18275 7921cf9

puppi isolation is added to pat muons in a similar was as it was done already for electrons; there is also a bugfix for electron puppi no-lepton iso
jenkins tests pass and comparisons show small changes in miniAOD electron puppi nolepton iso from the fix
local tests with a data file from DoubleMuon 2016E (wf 136.76) shows that isolation variables are filled with reasonably looking values. CPU cost for the two new modules muonPUPPIIsolation and muonPUPPINoLeptonsIsolation is somewhat high (5 ms/evt each, or 1% of total miniAOD time). It is expected to go down a bit after Muon arbitration #18321 is merged

jshlee added 5 commits March 20, 2017 02:40

debug CITK iso for muon

3264199

debug usePUPPINoLepton

55b1220

Merged muonPuppiIso from repository jshlee

ee34ad3

adding puppi iso for muons

159826b

bugfix

96e6205

cmsbuild added this to the Next CMSSW_9_1_X milestone Apr 9, 2017

cmsbuild added analysis-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 9, 2017

cmsbuild added tests-started and removed tests-pending labels Apr 9, 2017

cmsbuild added tests-approved and removed tests-started labels Apr 9, 2017

cmsbuild added comparison-available and removed comparison-pending labels Apr 9, 2017

slava77 reviewed Apr 15, 2017

View reviewed changes

slava's comments

7921cf9

removed _ from not member variables removed check of usePUPPINoLepton

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels Apr 18, 2017

cmsbuild added tests-started and removed tests-pending labels Apr 18, 2017

cmsbuild added tests-approved and removed tests-started labels Apr 18, 2017

cmsbuild added comparison-available and removed comparison-pending labels Apr 18, 2017

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Apr 18, 2017

davidlange6 merged commit e91d4d8 into cms-sw:master Apr 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Muon puppi iso miniAOD 91x #18275

Muon puppi iso miniAOD 91x #18275

jshlee commented Apr 9, 2017 •

edited

cmsbuild commented Apr 9, 2017

slava77 commented Apr 9, 2017

cmsbuild commented Apr 9, 2017 •

edited

cmsbuild commented Apr 9, 2017

cmsbuild commented Apr 9, 2017

cmsbuild commented Apr 9, 2017

jshlee commented Apr 10, 2017

slava77 Apr 15, 2017

jshlee Apr 18, 2017

slava77 Apr 15, 2017

slava77 Apr 17, 2017

jshlee Apr 18, 2017

slava77 Apr 15, 2017

slava77 Apr 15, 2017

jshlee Apr 18, 2017

slava77 commented Apr 17, 2017

jshlee commented Apr 18, 2017

cmsbuild commented Apr 18, 2017

slava77 commented Apr 18, 2017

cmsbuild commented Apr 18, 2017 •

edited

cmsbuild commented Apr 18, 2017

cmsbuild commented Apr 18, 2017

cmsbuild commented Apr 18, 2017

slava77 commented Apr 18, 2017

Muon puppi iso miniAOD 91x #18275

Muon puppi iso miniAOD 91x #18275

Conversation

jshlee commented Apr 9, 2017 • edited

cmsbuild commented Apr 9, 2017

slava77 commented Apr 9, 2017

cmsbuild commented Apr 9, 2017 • edited

cmsbuild commented Apr 9, 2017

cmsbuild commented Apr 9, 2017

cmsbuild commented Apr 9, 2017

jshlee commented Apr 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slava77 commented Apr 17, 2017

jshlee commented Apr 18, 2017

cmsbuild commented Apr 18, 2017

slava77 commented Apr 18, 2017

cmsbuild commented Apr 18, 2017 • edited

cmsbuild commented Apr 18, 2017

cmsbuild commented Apr 18, 2017

cmsbuild commented Apr 18, 2017

slava77 commented Apr 18, 2017

jshlee commented Apr 9, 2017 •

edited

cmsbuild commented Apr 9, 2017 •

edited

cmsbuild commented Apr 18, 2017 •

edited