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

PUPPI MET in RECO #29254

Merged
merged 20 commits into from
May 5, 2020
Merged

PUPPI MET in RECO #29254

merged 20 commits into from
May 5, 2020

Conversation

ahinzmann
Copy link
Contributor

@ahinzmann ahinzmann commented Mar 20, 2020

PR description:

This PR computes puppiNoLep weights and puppi MET in RECO and stores the puppiNoLep weight as a value map. PUPPI MET is clustered based on standard PF candidates together with the value map of weights. A new boolean "isWeighted" is added to the reco::PFMet interface to indicate whether weights were applied during jet clustering. The packed candidates pick up the value map such that puppi does not need to be re-computed in MiniAOD.

The same was done for the puppi and PUPPI jets in #28936.

Other changes:

  • Produce puppiForMET directly in PuppiProducer instead of a two step procedure puppiNoLep->puppiForMET and rename it to puppiNoLep, consistent with the name in PackedCandidates
  • Implement fillDescriptions for PFMetProducer
  • Rename PFMET_cfi to pfMet_cfi everywhere (for consistency with the module name)
  • Fix input jet collection of pfMetEI
  • Remove pfMetEI from RecoMET_EventContent (however, leave EITopPAG_EventContent_cff untouched)
  • Resolve issue cleanup miniAOD workflow after moving PUPPI to reco #29264
  • By moving to the same input PF candidate collection for puppi MET and puppi jets (with just different weights), PUPPI MET significance and unclustered pT computation then don't rely on dR matching as implemented in Fix MET unclustered energy computation #29331 anymore.
  • Store ref to PF candidate inside pat muons (just like done for all other PAT objects). PUPPI MET significance and unclustered pT computation then don't rely on dR matching as implemented in Fix MET Significance, Covariance Matrix and unclustered energy lepton subtraction #29385 anymore.
  • Fix particle multiplicity computation in xy-correction for PUPPI MET. Previously, particles with a weight of 0 could also count in the multiplicity as a function of which the xy-corrections are computed. Since xy-correction are not needed/recommended for PUPPI MET, this fix is not backported.
  • For HI workflows, raise candidate pT threshold for chsMet and trkMet to 999. consistent with pfMet.

PR validation:

runTheMatrix -l limited

if this PR is a backport please specify the original PR and why you need to backport that PR:

No backport foreseen.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@ahinzmann
Copy link
Contributor Author

please wait before testing, I hit enter too quickly

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29254/14294

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

CommonTools/ParticleFlow
CommonTools/PileupAlgos
DataFormats/METReco
PhysicsTools/PatAlgos
PhysicsTools/PatUtils
RecoMET/Configuration
RecoMET/METAlgorithms
RecoMET/METProducers

@perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @hatakeyamak, @emilbols, @peruzzim, @seemasharmafnal, @mmarionncern, @smoortga, @jdolen, @ferencek, @rovere, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @andrzejnovak, @clelange, @riga, @JyothsnaKomaragiri, @mverzett, @cbernet, @gpetruc, @mariadalfonso this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@slava77
Copy link
Contributor

slava77 commented Mar 20, 2020

@ahinzmann
will this need to be tested together with #28936 ?
Please clarify.
Thank you.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29254/14301

@cmsbuild
Copy link
Contributor

Pull request #29254 was updated. @perrotta, @cmsbuild, @santocch, @slava77 can you please check and sign again.

@ahinzmann
Copy link
Contributor Author

This can be tested standalone. Once #28936 is merged in CMSSW, I can do the merge here as well. I'm still finishing one round of local testing. Once they finish, it would be good to test here.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@ahinzmann
Copy link
Contributor Author

local test ran successful. Could this version be tested?

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29254/14303

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@jfernan2
Copy link
Contributor

+1

@slava77
Copy link
Contributor

slava77 commented Apr 29, 2020

+1

for #29254 d6e1687

  • code changes are in line with the PR description and the follow up review
  • jenkins tests pass and comparisons with the baseline show differences in expected places in puppi MET related variables (see earlier discussion)

resolves #29264

@silviodonato
Copy link
Contributor

Do you have comments @santocch ?

@slava77
Copy link
Contributor

slava77 commented May 5, 2020

@silviodonato
please include this in pre7

@silviodonato
Copy link
Contributor

merge

@silviodonato
Copy link
Contributor

We are getting some errors in the IB. Please look at #29744

@santocch
Copy link

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged.

Comment on lines 836 to 838
if not self.getvalue("onMiniAOD"):
pfCandsNoJetsNoEle.useDeltaRforFootprint = True
pfCandsNoJetsNoEle.veto = "pfeGammaToCandidate:electrons"
addToProcessAndTask("pfCandsNoJetsNoEle"+postfix, pfCandsNoJetsNoEle, process, task)
Copy link
Contributor

Choose a reason for hiding this comment

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

I was debugging differences in UL re-miniAOD between 106X and 112X, and was puzzled by the somewhat small but numerous changes in slimmedMETsNoHF UnclusteredEnUp/Down.
If I understood correctly (although I did not add any debug printouts to confirm), the problem is that the "NoHF" is based on noHFCands, which are copies (not Ptrs) of the original PF cands

if (selector_(*pfc)) {
selected_.push_back(reco::PFCandidate(*pfc));
reco::PFCandidatePtr ptrToMother(hc, key);
selected_.back().setSourceCandidatePtr(ptrToMother);

So, the CandPtrProjector can not match the vetoing electron's sourceCandidatePtr (the original particleFlow) to the noHFCands (another check by the cand's sourceCandidatePtr may suffice though).
So, either we need an extension in CandPtrProjector to also check for the main collection sourceCandidatePtr or instead have useDeltaRforFootprint = True

I'll re-post this as an issue.

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.

8 participants