Fix PUPPI MET tails #17072

ahinzmann · 2016-12-19T12:34:56Z

This contains 2 fixes needed to remove tails of the PUPPI MET distribution:

Photon ID was not applied in PUPPI Photon code
PUPPI photon code was not protected against adding multiple copies of the same PF candidate in the output collected
The impact of this PR on jet and MET response and resolution has been validated.
Slide 17-22 show the impact of this change to MET:
https://indico.cern.ch/event/587258/contributions/2366495/attachments/1368938/2117761/satoshi_updatedAfterTheMeeting.pdf
What is labeled v10 is in the CMSSW_8_0_>=20 and CMSSW_8_1_X and CMSSW_9_0_X releases at the moment.
What is labeled "PFPUPPI v10 PHfix wgtRecal" is in this PR.

This PR changes the MINIAOD PUPPI MET collection, no changes to RECO/AOD event content.

cmsbuild · 2016-12-19T12:35:15Z

A new Pull Request was created by @ahinzmann for CMSSW_9_0_X.

It involves the following packages:

CommonTools/PileupAlgos
PhysicsTools/PatAlgos

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

cms-bot commands are listed here #13028

slava77 · 2016-12-19T14:00:31Z

@cmsbuild please test

cmsbuild · 2016-12-19T14:00:45Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17108/console Started: 2016/12/19 15:02

cmsbuild · 2016-12-19T15:28:41Z

+1
Tested at: 018386b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17072/17108/summary.html

cmsbuild · 2016-12-19T15:28:44Z

Comparison job queued.

cmsbuild · 2016-12-19T17:16:26Z

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

slava77 · 2016-12-29T18:39:11Z

CommonTools/PileupAlgos/plugins/PuppiPhoton.cc

@@ -31,17 +31,19 @@ PuppiPhoton::PuppiPhoton(const edm::ParameterSet& iConfig) {
  tokenPFCandidates_     = consumes<CandidateView>(iConfig.getParameter<edm::InputTag>("candName"));
  tokenPuppiCandidates_  = consumes<CandidateView>(iConfig.getParameter<edm::InputTag>("puppiCandName"));
  tokenPhotonCandidates_ = consumes<CandidateView>(iConfig.getParameter<edm::InputTag>("photonName"));
-  tokenPhotonId_         = consumes<edm::ValueMap<bool>  >(iConfig.getParameter<edm::InputTag>("photonId")); 
+  reco2pf_               = mayConsume<edm::ValueMap<std::vector<reco::PFCandidateRef> > >(iConfig.getParameter<edm::InputTag>("recoToPFMap"));


please change to consumes according to the value of runOnMiniAOD.
The current implementation has no alternatives if runOnMiniAOD is false.

slava77 · 2016-12-29T18:44:32Z

CommonTools/PileupAlgos/plugins/PuppiPhoton.cc

@@ -31,17 +31,19 @@ PuppiPhoton::PuppiPhoton(const edm::ParameterSet& iConfig) {
  tokenPFCandidates_     = consumes<CandidateView>(iConfig.getParameter<edm::InputTag>("candName"));
  tokenPuppiCandidates_  = consumes<CandidateView>(iConfig.getParameter<edm::InputTag>("puppiCandName"));
  tokenPhotonCandidates_ = consumes<CandidateView>(iConfig.getParameter<edm::InputTag>("photonName"));
-  tokenPhotonId_         = consumes<edm::ValueMap<bool>  >(iConfig.getParameter<edm::InputTag>("photonId")); 
+  reco2pf_               = mayConsume<edm::ValueMap<std::vector<reco::PFCandidateRef> > >(iConfig.getParameter<edm::InputTag>("recoToPFMap"));
+  tokenPhotonId_         = mayConsume<edm::ValueMap<bool>  >(iConfig.getParameter<edm::InputTag>("photonId"));


please change to consumes based on the value of usePhotonId.
Actually, if photonId is empty, the framework will already ignore the consumes

slava77 · 2016-12-29T19:05:36Z

CommonTools/PileupAlgos/plugins/PuppiPhoton.cc

+      const pat::Photon *pPho = dynamic_cast<const pat::Photon*>(&(*itPho));
+      if(pPho != 0) {
+        for( const edm::Ref<pat::PackedCandidateCollection> & ref : pPho->associatedPackedPFCandidates() ) {
+	  if(fabs(pfCol->ptrAt(ref.key())->eta()) < eta_ ) {


some comments would be useful to say that for "runOnMiniAOD" the pfCol is supposed to be a packedcandidate collection.
Better yet, use ref directly, since unless pfCol->ptrAt(ref.key()) points to the same object as ref, there is a problem.

slava77 · 2016-12-29T19:18:44Z

CommonTools/PileupAlgos/plugins/PuppiPhoton.cc

+      }
+    } else {
+      for( const edm::Ref<std::vector<reco::PFCandidate> > & ref : (*reco2pf)[phoCol->ptrAt(iC)] ) {
+	  if(fabs(pfCol->ptrAt(ref.key())->eta()) < eta_ )  {


here again it looks like pfCol->ptrAt(ref.key()) should be replaced with ref, else this opens up a way for inconsistencies

slava77 · 2016-12-30T02:16:44Z

Here are some comparisons on top of #17067
The baseline (black) is 900pre2 + #17067 using 900pre2 RECO files as an inputs; the "new" is this PR + #17067

HGG PU35, in the standard AOD->MINIAOD workflow

since the genMET is essentially zero, the diff with genmet shows an increase in the over-measured values

e.g. the slice of MET 80 to 100 more than doubles in population

I'm guessing it's the ID cuts which are now applied
$wf25203_7avs6_puppimet_photonetfrac$

HGG no PU

ZEE PU35 and TTbar PU35 do not have a significant change.

=============

so, this PR makes PUPPI MET worse for events with photons.
The increase in MET tail from this PR itself in events with photons is slightly worse in no-PU (the improvements in #17067 balance out the degradation in MET from this PR).

Looking at the fix referenced in the slides
cms-met/cmssw@METRecipe_8020...ahinzmann:METRecipe_8020_Moriond17
I notice only the difference in ID for the code part.

The slides suggest that puppiMET is remade from miniAOD inputs.
It looks like we are again hitting the problem that the code is developed and debugged based on rerunning on top of miniAOD and it behaves differently compared to running on AOD (the default puppi and other miniAOD products).
This kind of defeats the purpose of having puppi in standard miniAOD outputs.

Please make some plots comparing puppiMET made from AOD and that made from miniAOD.
If there is decent agreement, maybe the ID is to blame and it could be possibly updated to a more recent one.

cmsbuild · 2017-01-06T20:17:00Z

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

…rom AOD and MiniAOD more similar

cmsbuild · 2017-01-06T21:41:00Z

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

ahinzmann · 2017-01-06T21:57:05Z

@slava77 I implemented the comments and did a comparison of running on AOD and MiniAOD. Starting from 1000 event in
/store/relval/CMSSW_9_0_0_pre2/RelValH125GGgluonfusion_13/GEN-SIM-RECO/PU25ns_90X_mcRun2_asymptotic_v0-v1/10000/167B6FB3-B4C2-E611-AAEE-0CC47A7C3410.root
I ran the MiniAOD sequence from cmsDriver.
Then reran PUPPI on top of this MiniAOD.
I also enabled "useExistingWeights" when rerunning PUPPI to minimize effects of numerical precision.
The resulting comparison is attached.

The MET value when running on AOD and MiniAOD agrees within 1% for all events. For example, the first couple of events differ like this:

< PUPPI MET: pt 8.1

PUPPI MET: pt 8.2
258c258
< PUPPI MET: pt 23.5

PUPPI MET: pt 23.1
261c261
< PUPPI MET: pt 14.0

PUPPI MET: pt 14.1
264c264
< PUPPI MET: pt 28.4

PUPPI MET: pt 28.5
267c267
< PUPPI MET: pt 53.5

PUPPI MET: pt 53.7
285c285
< PUPPI MET: pt 33.5

PUPPI MET: pt 33.3
288c288
< PUPPI MET: pt 15.9

PUPPI MET: pt 15.8
291c291
< PUPPI MET: pt 11.7

PUPPI MET: pt 11.8
300c300
< PUPPI MET: pt 47.4

PUPPI MET: pt 47.5

So the behavior you observe does not come from AOD/MiniAOD differences. I also doubt it comes from the photon ID, since everything was tested with the Spring15 photon ID that is also in CMSSW_9_0_0. The change of the METRecipe_8020_Moriond17 recipe to the Spring16 photon ID only happened recently with the new Moriond recommendations.

Do you think you could check one more plot, to confirm that this PR achieves its main purpose, that is removing the MET tails in QCD events as shown on slide 3 of https://indico.cern.ch/event/587258/contributions/2366495/attachments/1368938/2117761/satoshi_updatedAfterTheMeeting.pdf
A plot of the MET in log-y-scale in a flat (or high pT) QCD sample should show this.
It could be possible that getting rid of these tails trades off with some performance loss in the gamma gamma MET.

slava77 · 2017-01-06T22:17:36Z

On 1/6/17 1:57 PM, ahinzmann wrote: Do you think you could check one more plot, to confirm that this PR achieves its main purpose, that is removing the MET tails in QCD events as shown on slide 3 of https://indico.cern.ch/event/587258/contributions/2366495/attachments/1368938/2117761/satoshi_updatedAfterTheMeeting.pdf A plot of the MET in log-y-scale in a flat (or high pT) QCD sample should show this. It could be possible that getting rid of these tails trades off with some performance loss in the gamma gamma MET.

I can check in the QCD sample, but mostly likely not until Monday, possibly during the weekend. Just to be more specific on the sample: which sample is used in p3? The text says "flat QCD (MiniAOD one file)." Is there PU in this sample? p5 has /QCD_Pt-15to7000_TuneCUETP8M1_Flat_13TeV_pythia8/RunIISummer16MiniAODv2-PUFlat0to70_magnetOn_80X_mcRun2_asymptotic_2016_TrancheIV_v4-v1/MINIAODSIM with pileup Was it this sample?

ahinzmann · 2017-01-06T22:34:58Z

@slava77 confirmed. The 0-70 PU sample. (Though PU is not the source of the tails, thus the MET tails should show up in any flat/high pT QCD samples)

slava77 · 2017-01-09T22:08:06Z

@cmsbuild please test

cmsbuild · 2017-01-09T22:08:22Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17221/console Started: 2017/01/09 23:09

cmsbuild · 2017-01-09T23:31:27Z

+1
Tested at: acd025b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17072/17221/summary.html

cmsbuild · 2017-01-09T23:31:30Z

Comparison job queued.

cmsbuild · 2017-01-10T01:31:36Z

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

slava77 · 2017-01-12T05:54:15Z

Here is a diff for 25209.0 miniAOD (from RECO /RelValQCD_FlatPt_15_3000HS_13/CMSSW_9_0_0_pre2-PU25ns_90X_mcRun2_asymptotic_v0-v1/GEN-SIM-RECO )
900pre2 as a baseline (black) and this 900pre2+this PR cherry-picked (in red)

clearly, there is a reduction of large tails.
This appears to come with some broadening of the core, which is slightly visible in the plot above as well as in the slide 3 from
https://indico.cern.ch/event/587258/contributions/2366495/attachments/1368938/2117761/satoshi_updatedAfterTheMeeting.pdf

So, the Hgammagamma plots I posted earlier are also consistent.

@ahinzmann
Based on the context and your confirmation of MET remade from miniAOD and from AOD, it looks like this can be signed off.

slava77 · 2017-01-12T06:05:23Z

+1

for #17072 acd025b

code changes are in line with the description and follow up review
jenkins test pass and comparisons show changes only in puppi-related variables
results of tests with larger samples were posted earlier

ahinzmann added 2 commits December 19, 2016 10:37

fix puppi photon with no lepton collection

b7a4262

recompute weights

018386b

cmsbuild added this to the Next CMSSW_9_0_X milestone Dec 19, 2016

cmsbuild added analysis-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Dec 19, 2016

This was referenced Dec 19, 2016

Fix PUPPI MET tails cms-met/cmssw#18

Merged

Fix PUPPI MET tails #17073

Closed

cmsbuild added tests-started and removed tests-pending labels Dec 19, 2016

cmsbuild added tests-approved and removed tests-started labels Dec 19, 2016

cmsbuild added comparison-available and removed comparison-pending labels Dec 19, 2016

slava77 reviewed Dec 29, 2016

View reviewed changes

get rid of mayConsume

a17080a

cmsbuild removed the comparison-available label Jan 6, 2017

cmsbuild added comparison-pending tests-pending and removed tests-approved labels Jan 6, 2017

use existing weights when running on top of MiniAOD to make results f…

acd025b

…rom AOD and MiniAOD more similar

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

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

cmsbuild added comparison-available and removed comparison-pending labels Jan 10, 2017

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jan 12, 2017

davidlange6 merged commit 7644e7d into cms-sw:CMSSW_9_0_X Jan 12, 2017

slava77 mentioned this pull request May 2, 2017

Fix for Puppi MET and MET significance compatibility between AOD and miniAOD (80X), rebased #18341

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix PUPPI MET tails #17072

Fix PUPPI MET tails #17072

ahinzmann commented Dec 19, 2016

cmsbuild commented Dec 19, 2016

slava77 commented Dec 19, 2016

cmsbuild commented Dec 19, 2016 •

edited

cmsbuild commented Dec 19, 2016

cmsbuild commented Dec 19, 2016

cmsbuild commented Dec 19, 2016

slava77 Dec 29, 2016

slava77 Dec 29, 2016

slava77 Dec 29, 2016

slava77 Dec 29, 2016

slava77 commented Dec 30, 2016

cmsbuild commented Jan 6, 2017

cmsbuild commented Jan 6, 2017

ahinzmann commented Jan 6, 2017

slava77 commented Jan 6, 2017 via email

ahinzmann commented Jan 6, 2017

slava77 commented Jan 9, 2017

cmsbuild commented Jan 9, 2017 •

edited

cmsbuild commented Jan 9, 2017

cmsbuild commented Jan 9, 2017

cmsbuild commented Jan 10, 2017

slava77 commented Jan 12, 2017

slava77 commented Jan 12, 2017

Fix PUPPI MET tails #17072

Fix PUPPI MET tails #17072

Conversation

ahinzmann commented Dec 19, 2016

cmsbuild commented Dec 19, 2016

slava77 commented Dec 19, 2016

cmsbuild commented Dec 19, 2016 • edited

cmsbuild commented Dec 19, 2016

cmsbuild commented Dec 19, 2016

cmsbuild commented Dec 19, 2016

slava77 Dec 29, 2016

Choose a reason for hiding this comment

slava77 Dec 29, 2016

Choose a reason for hiding this comment

slava77 Dec 29, 2016

Choose a reason for hiding this comment

slava77 Dec 29, 2016

Choose a reason for hiding this comment

slava77 commented Dec 30, 2016

cmsbuild commented Jan 6, 2017

cmsbuild commented Jan 6, 2017

ahinzmann commented Jan 6, 2017

< PUPPI MET: pt 8.1

slava77 commented Jan 6, 2017 via email

ahinzmann commented Jan 6, 2017

slava77 commented Jan 9, 2017

cmsbuild commented Jan 9, 2017 • edited

cmsbuild commented Jan 9, 2017

cmsbuild commented Jan 9, 2017

cmsbuild commented Jan 10, 2017

slava77 commented Jan 12, 2017

slava77 commented Jan 12, 2017

cmsbuild commented Dec 19, 2016 •

edited

cmsbuild commented Jan 9, 2017 •

edited