Fix in puppi photon matching #19587
Fix in puppi photon matching #19587
Conversation
|
A new Pull Request was created by @ahinzmann for master. It involves the following packages: CommonTools/PileupAlgos @perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
On 7/6/17 7:26 AM, ahinzmann wrote:
PUPPI MET in HT>600 gamma+jet sample without fix (tested in CMSSW_8_0_X)
were the tests made on top of #18341 ?
this would represent expected performance in 9X.
If that was not the case, it would be rather useful to see the
comparison in 93X itself.
|
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
| @@ -186,6 +186,7 @@ void PuppiPhoton::produce(edm::Event& iEvent, const edm::EventSetup& iSetup) { | |||
| } | |||
| // ------------------------------------------------------------------------------------------ | |||
| bool PuppiPhoton::matchPFCandidate(const reco::Candidate *iPF,const reco::Candidate *iPho) { | |||
| if(iPF->pdgId() != iPho->pdgId()) return false; | |||
| double lDR = deltaR(iPF->eta(),iPF->phi(),iPho->eta(),iPho->phi()); | |||
| for(unsigned int i0 = 0; i0 < pdgIds_.size(); i0++) { | |||
| if(std::abs(iPF->pdgId()) == pdgIds_[i0] && lDR < dRMatch_[i0]) return true; | |||
There was a problem hiding this comment.
the change above apparently at least partially invalidates the logic here, where other pdgIds are checked specifically.
Should the loop over pdgIds_ be removed?
I thought that matching of a photon to an electron were still meaningful, but maybe I don't have a good memory of what is done in PuppiPhoton
There was a problem hiding this comment.
the matching is to make sure it is the vary same PF candidate, so the same PDG ID is required. So electrons can be matched to electrons and photons to photons, but not mixed.
The loop over pdg id allows to configure different dR criteria for different particle types.
|
@slava77 Does some 9_3_X-compatible HT>600 gamma+jet sample happen to exist to repeat the test? |
|
On 7/6/17 7:49 AM, ahinzmann wrote:
@slava77 <https://github.com/slava77> Does some 9_3_X-compatible HT>600
gamma+jet sample happen to exist to repeat the test?
RelValPhotonJets_Pt_10_13 is available
perhaps is not high enough in HT.
We probably had more relevant samples in 902 production.
It should also be possible to run miniAOD in 93X or 92X on 80X inputs
Check the setup in the matrix workflow in 1325.5
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#19587 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbsay60ow7lwjbqDq2lSn5Rt-LOsmks5sLPPlgaJpZM4OPtri>.
|
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
Here are tests on a flat pT>200 GeV gamma+jets sample (test tails of MET distribution) and a flat pT>15 GeV QCD sample (test bulk of MET distribution). The fix in the matching procedure is a minor effect, since the bug was almost completely fixed by #18341 already However, high MET tails are visible, which are due to three problems.
Therefore the PR was adapted to remove the photon ID requirement and use all PF photons and raise the photon pt cut from 10 GeV to 20 GeV to compensate for this. This was found to remove the photon-related MET tails in the photon+jets samples and keep the MET bulk in the QCD sample similar. This can be considered as the best we can do while 2017 egamma IDs are not available. It is better to have PUPPI MET working independent of the egamma IDs, rather than waiting for the last moment when full scale validation of the impact will be difficult. Black= PF MET high pt Photon+jet samples low pt QCD sample
This can be fixed by adding a protection to the PUPPI algorithm to not reject high momentum neutral particles. This was implemented by gradually increasing the PUPPI weight from 0 to 1 from 0 to 200 GeV if this is larger than the weight given by the PUPPI algorithm. Black= PF MET high pt Photon+jet samples low pt QCD sample @violatingcp tested the same change on top of 80X with high statistics DY and HT>600 gamma+jets. Red=PF MET low pT DY samples high pT gamma+jets @slava77 Since this became more than a simple bug fix, we'll have to discuss this in JetMET before this can be merged. |
| @@ -76,7 +71,25 @@ void PuppiPhoton::produce(edm::Event& iEvent, const edm::EventSetup& iSetup) { | |||
| edm::Handle<CandidateView> hPuppiProduct; | |||
| iEvent.getByToken(tokenPuppiCandidates_,hPuppiProduct); | |||
| const CandidateView *pupCol = hPuppiProduct.product(); | |||
| for(CandidateView::const_iterator itPho = phoCol->begin(); itPho!=phoCol->end(); itPho++) { | |||
| if(usePFphotons_) { | |||
| for(CandidateView::const_iterator itPho = pfCol->begin(); itPho!=pfCol->end(); itPho++) { | |||
There was a problem hiding this comment.
hi @ahinzmann - this can instead be a range based loop
for ( const auto & pho : pfCol ) {...
if pho.pt() < pt_ continue...
There was a problem hiding this comment.
this gives me a
"/afs/cern.ch/user/h/hinzmann/stable_13TeV/jetmet/CMSSW_9_3_0_pre1/src/CommonTools/PileupAlgos/plugins/PuppiPhoton.cc: In member function 'virtual void PuppiPhoton::produce(edm::Event&, const edm::EventSetup&)':
/afs/cern.ch/user/h/hinzmann/stable_13TeV/jetmet/CMSSW_9_3_0_pre1/src/CommonTools/PileupAlgos/plugins/PuppiPhoton.cc:75:27: error: 'begin' was not declared in this scope
for(const auto & pho : pfCol) {
"
what's the exact syntax for
const CandidateView *pfCol = hPFProduct.product();
There was a problem hiding this comment.
for ( const auto & pho : *pfCol )
| for(CandidateView::const_iterator itPho = pfCol->begin(); itPho!=pfCol->end(); itPho++) { | ||
| iC++; | ||
| if(itPho->pt() < pt_) continue; | ||
| if(abs(itPho->pdgId())!=22) continue; |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1
|
|
merge |
Protect PUPPI photon matching against other PF candidate types that may overlap with photons in angular space.
This reduces the PUPPI MET tail in gamma+jets events.
Validation plots and further details given in the discussion of this PR below.