Bug fix for track MET in MET correction tools #34860

lathomas · 2021-08-12T18:39:33Z

PR description:

This PR is a fix of a longstanding bug in the calculation of the track MET in the MET correction tools that results in a wrong value in NANOAOD.
The bug was recently (re?)discovered when trying to run MINI and NANO in a single job (see #34714 )

The PF candidate filter is now in synch with the one used in the MINIAOD tools.

https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py#L219-L222

PR validation:

Explicitly checked that the TrackMET value in NANOAOD is identical when you fetch the MINIAOD value or recompute it using the MET correction tool.

cmsbuild · 2021-08-12T18:46:25Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34860/24658

This PR adds an extra 32KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File PhysicsTools/PatUtils/python/tools/runMETCorrectionsAndUncertainties.py modified in PR(s): Unify vertex association between CHS and PUPPI #33885

cmsbuild · 2021-08-12T18:46:49Z

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

It involves the following packages:

PhysicsTools/PatUtils (reconstruction)

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @gouskos, @JyothsnaKomaragiri, @ahinzmann, @schoef, @rappoccio, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @clelange, @emilbols, @hatakeyamak, @gpetruc, @andrzejnovak, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

slava77 · 2021-08-12T18:54:19Z

@cmsbuild please test

cmsbuild · 2021-08-12T22:08:39Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-049747/17748/summary.html
COMMIT: f45c5fb
CMSSW: CMSSW_12_1_X_2021-08-12-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34860/17748/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 11 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 2999420
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2999391
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

slava77 · 2021-08-13T00:24:32Z

PhysicsTools/PatUtils/python/tools/runMETCorrectionsAndUncertainties.py

@@ -1728,7 +1728,7 @@ def miniAODConfigurationPre(self, process, patMetModuleSequence, pfCandCollectio
        patMetModuleSequence += getattr(process, "pfMetCHS")
        patMetModuleSequence += getattr(process, "patCHSMet")

-        pfTrk = cms.EDFilter("CandPtrSelector", src = cms.InputTag("packedPFCandidates"), cut = cms.string("fromPV(0) > 0 && charge()!=0"))
+        pfTrk = cms.EDFilter("CandPtrSelector", src = cms.InputTag("packedPFCandidates"), cut = cms.string("pvAssociationQuality()>=4 && charge()!=0 && vertexRef().key()==0"))


if this has to be the same as in miniAOD_tools.py, please define a common reference and use it or clone from it directly in the places where these need to be the same.

Here, it looks like the package dependency chain suggests this directory is OK. So, e.g. in PhysicsTools/PatUtils/python/tools/trackSelectors_cff.py

chargedPackedCandsForMet = cms.EDFilter("CandPtrSelector" ...

would be great to do as well for the pfCHS.
note in runMETCorrectionsAndUncertainties.py is even defined twice once for jets and once for met

cmssw/PhysicsTools/PatUtils/python/tools/runMETCorrectionsAndUncertainties.py

Line 1713 in f45c5fb

pfCHS = cms.EDFilter("CandPtrSelector", src = cms.InputTag("packedPFCandidates"), cut = cms.string("fromPV(0)>0"))

cmssw/PhysicsTools/PatUtils/python/tools/runMETCorrectionsAndUncertainties.py

Line 1610 in f45c5fb

pfCHS = cms.EDFilter("CandPtrSelector", src = pfCandCollection, cut = cms.string("fromPV"))

The duplicate definition of pfCHS is adressed in #33885.
@lathomas: Could you take care of defining the trkMET selection as suggested by Slava (and I sync to that once you're done)?

slava77 · 2021-08-20T12:39:11Z

@lathomas
please clarify on the status of updating this PR following the review.
Thank you.

slava77 · 2021-08-23T13:52:29Z

@lathomas @ahinzmann
will there be some interference between this PR and #33885?

more details about #33885 were covered in the reco meeting
https://indico.cern.ch/event/1067481/contributions/4492388/attachments/2296238/3905408/20210604_trackvertexassociation.pdf

ahinzmann · 2021-08-23T13:56:08Z

@lathomas @ahinzmann
will there be some interference between this PR and #33885?

more details about #33885 were covered in the reco meeting
https://indico.cern.ch/event/1067481/contributions/4492388/attachments/2296238/3905408/20210604_trackvertexassociation.pdf

@lathomas #33885 could potentially also address the issue tackled by this PR. Are you able to test if #33885 helps cure the issue here?

lathomas · 2021-08-23T14:04:26Z

Hello.
No. It looks like TrkMET is still computed in the wrong way in #33885
This line is incorrect:

cmssw/PhysicsTools/PatUtils/python/tools/runMETCorrectionsAndUncertainties.py

Line 1718 in 315f88f

    
           pfTrk = cms.EDFilter("CandPtrSelector", src = cms.InputTag("pfCHS"), cut = cms.string("charge()!=0"))

The input collection for TrkMET is not equivalent to charged pfCHS

ahinzmann · 2021-08-23T14:16:05Z

Hello.
No. It looks like TrkMET is still computed in the wrong way in #33885
This line is incorrect:

cmssw/PhysicsTools/PatUtils/python/tools/runMETCorrectionsAndUncertainties.py

Line 1718 in 315f88f

pfTrk = cms.EDFilter("CandPtrSelector", src = cms.InputTag("pfCHS"), cut = cms.string("charge()!=0"))

The input collection for TrkMET is not equivalent to charged pfCHS

What exactly is the difference? Is it a difference choice of pvAssociationQuality threshold?

lathomas · 2021-08-23T14:27:24Z

Yes. Track MET uses a tighter selection than CHS apparently:
https://github.com/cms-sw/cmssw/pull/34860/files#diff-79860e2d37e9cc0867e9de61faab9b79e0283ebf986856b7a05f517e6d1d083aL1731-R1731

cmsbuild · 2021-08-25T09:50:21Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34860/24862

This PR adds an extra 40KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py modified in PR(s): Unify vertex association between CHS and PUPPI #33885
- File PhysicsTools/PatUtils/python/tools/runMETCorrectionsAndUncertainties.py modified in PR(s): Unify vertex association between CHS and PUPPI #33885

cmsbuild · 2021-08-25T09:50:40Z

Pull request #34860 was updated. @jpata, @cmsbuild, @slava77 can you please check and sign again.

mariadalfonso · 2021-08-25T10:44:09Z

please test

cmsbuild · 2021-08-25T14:07:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-049747/18021/summary.html
COMMIT: 0e55061
CMSSW: CMSSW_12_1_X_2021-08-24-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34860/18021/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 13 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000352
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3000324
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

slava77 · 2021-08-25T14:33:55Z

+reconstruction

for #34860 0e55061

code changes are in line with the PR description and the follow up review: track-MET in NANO and mini is now defined using a common track selector
jenkins tests pass and comparisons with the baseline show differences in the nanoAOD tkMetTable; sumEt has the most clearly visible change showing a reduction in the selected tracks going to the sum e.g. in wf 136.8523 (JetHT2018)

cmsbuild · 2021-08-25T14:34:14Z

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 now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

slava77 · 2021-08-26T14:49:24Z

@perrotta @qliphy
This PR is holding up #33885 .
please check if this PR can be merged.

Thank you.

perrotta · 2021-08-26T15:01:34Z

@perrotta @qliphy
This PR is holding up #33885 .
please check if this PR can be merged.

Thank you.

@slava77 we are building pre2 right now.
This PR will get merged as soon as the build finishes succesfully, and we can release that pre

perrotta · 2021-08-27T13:27:55Z

+1

Bug fix for track MET in miniAOD tools

f45c5fb

cmsbuild added this to the CMSSW_12_1_X milestone Aug 12, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Aug 12, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 12, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 12, 2021

cmsbuild added tests-approved and removed tests-started labels Aug 12, 2021

slava77 reviewed Aug 13, 2021

View reviewed changes

mariadalfonso mentioned this pull request Aug 13, 2021

tk met cms-nanoAOD/cmssw#10

Closed

cmsbuild mentioned this pull request Aug 16, 2021

Unify vertex association between CHS and PUPPI #33885

Merged

Defining a common module for pfcands used in trk met

0e55061

cmsbuild added code-checks-pending and removed tests-approved code-checks-approved labels Aug 25, 2021

cmsbuild added the tests-pending label Aug 25, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 25, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 25, 2021

cmsbuild added tests-approved and removed tests-started labels Aug 25, 2021

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Aug 25, 2021

cmsbuild mentioned this pull request Aug 25, 2021

Adapt CAConstants for HIon #34811

Closed

cmsbuild added orp-approved and removed orp-pending labels Aug 27, 2021

cmsbuild merged commit 1cbe017 into cms-sw:master Aug 27, 2021

cmsbuild mentioned this pull request Aug 27, 2021

Removed undefined use of union in libminifloat #35054

Merged

mariadalfonso mentioned this pull request Sep 28, 2021

new features for V10 nanoAOD cms-nanoAOD/cmssw#580

Closed

39 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug fix for track MET in MET correction tools #34860

Bug fix for track MET in MET correction tools #34860

lathomas commented Aug 12, 2021

cmsbuild commented Aug 12, 2021

cmsbuild commented Aug 12, 2021

slava77 commented Aug 12, 2021

cmsbuild commented Aug 12, 2021

slava77 Aug 13, 2021

mariadalfonso Aug 13, 2021 •

edited

ahinzmann Aug 23, 2021

slava77 commented Aug 20, 2021

slava77 commented Aug 23, 2021

ahinzmann commented Aug 23, 2021

lathomas commented Aug 23, 2021

ahinzmann commented Aug 23, 2021

lathomas commented Aug 23, 2021

cmsbuild commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

mariadalfonso commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

slava77 commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

slava77 commented Aug 26, 2021

perrotta commented Aug 26, 2021

perrotta commented Aug 27, 2021

Bug fix for track MET in MET correction tools #34860

Bug fix for track MET in MET correction tools #34860

Conversation

lathomas commented Aug 12, 2021

PR description:

PR validation:

cmsbuild commented Aug 12, 2021

cmsbuild commented Aug 12, 2021

slava77 commented Aug 12, 2021

cmsbuild commented Aug 12, 2021

Comparison Summary

slava77 Aug 13, 2021

Choose a reason for hiding this comment

mariadalfonso Aug 13, 2021 • edited

Choose a reason for hiding this comment

ahinzmann Aug 23, 2021

Choose a reason for hiding this comment

slava77 commented Aug 20, 2021

slava77 commented Aug 23, 2021

ahinzmann commented Aug 23, 2021

lathomas commented Aug 23, 2021

ahinzmann commented Aug 23, 2021

lathomas commented Aug 23, 2021

cmsbuild commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

mariadalfonso commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

Comparison Summary

slava77 commented Aug 25, 2021

cmsbuild commented Aug 25, 2021

slava77 commented Aug 26, 2021

perrotta commented Aug 26, 2021

perrotta commented Aug 27, 2021

mariadalfonso Aug 13, 2021 •

edited