Unify vertex association between CHS and PUPPI #33885

ahinzmann · 2021-05-28T15:48:54Z

PR description:

Track-vertex association handling is quite inhomogeneous across CMSSW.
While the ultimate goal is to fully unify vertex association across CHS and PUPPI for Run3 and the use of timing information in CHS and PUPPI for Phase-2, this PR enables technically to study+synchronize them, while not significantly changing the current behavior of CHS and PUPPI.

Use a single module in CMSSW for all track-vertex association done for PackedCandidates and CHS and PUPPI:
"CommonTools/RecoAlgos/src/PrimaryVertexAssignment.cc"
Move "PhysicsTools.PatAlgos.slimming.primaryVertexAssociation_cfi" to "CommonTools.RecoAlgos.primaryVertexAssociation_cfi"
Remove usage of fromPV for CHS and PUPPI, and instead make use of a common track vertex association map in a common config file:
"packedPrimaryVertexAssociationJME"
"CommonTools/ParticleFlow/python/pfCHS_cff.py"
Enable CHS to use advanced track-vertex association via a map like PackedCandidates and PUPPI, via two producers at AOD and MiniAOD level:
"CommonTools/ParticleFlow/plugins/PFPileUp.cc" (AOD)
"CommonTools/ParticleFlow/plugins/PFNoPileUpPacked.cc" (MiniAOD)
The behavior of CHS is slightly modified by applying a criterion to recover cases where the primary vertex is split in sync with PUPPI (Now L71 in CommonTools/RecoAlgos/src/PrimaryVertexAssignment.cc). In this way, vertex association based on UsedInFit is the same for PUPPI and CHS and they only differ (until further unified) in additional requirements based on dz for PUPPI.

Performance tested in CMSSW_11_1_8 using HLT-TDR samples with this backport:
https://github.com/cms-sw/cmssw/compare/CMSSW_11_1_X...ahinzmann:primaryVertexAssociationJME?expand=1
Validation presented at RECO on Aug 20th and PF meetings on June 4th:
https://indico.cern.ch/event/1067481/#67-unified-vertex-association
https://indico.cern.ch/event/1045967/#40-track-primary-vertex-associ

PR validation:

scram b runtests

runTheMatrix -l limited

MiniAOD output of 25202.0 CHS jets were found to agree 100%. PUPPI jets were found to agree within <1%.

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

No backport planned.
Will provide a synchronized analysis recipe for studies based on HLT-TDR samples.

cmsbuild · 2021-05-28T15:54:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33885/22925

This PR adds an extra 84KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CommonTools/RecoAlgos/plugins/PrimaryVertexSorter.h modified in PR(s): [RFC] Test clang-tidy --checks misc-definitions-in-headers #33366
- File PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py modified in PR(s): Add Muon L1 to prefire weight producer #33758
- File PhysicsTools/PatAlgos/python/slimming/slimming_cff.py modified in PR(s): Adding offlineVerticesWithBS to miniAOD #33778

cmsbuild · 2021-05-28T15:54:55Z

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

It involves the following packages:

CommonTools/ParticleFlow
CommonTools/PileupAlgos
CommonTools/RecoAlgos
PhysicsTools/PatAlgos
PhysicsTools/PatUtils
RecoBTag/ONNXRuntime
RecoJets/JetProducers

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

cms-bot commands are listed here

slava77 · 2021-05-29T18:40:46Z

@ahinzmann
there are merge conflicts in PhysicsTools/PatAlgos/python/slimming/ primaryVertexAssociation_cfi and slimming_cff files

cmsbuild · 2021-05-29T21:38:06Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33885/22954

This PR adds an extra 212KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CommonTools/RecoAlgos/plugins/PrimaryVertexSorter.h modified in PR(s): [RFC] Test clang-tidy --checks misc-definitions-in-headers #33366
- File PhysicsTools/PatAlgos/python/slimming/lostTracks_cfi.py modified in PR(s): Adding Xi-compatible tracks to lostTracks collection #33891
- File PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py modified in PR(s): Add Muon L1 to prefire weight producer #33758

cmsbuild · 2021-05-29T21:38:27Z

Pull request #33885 was updated. @perrotta, @andrius-k, @jordan-martins, @kmaeshima, @wajidalikhan, @ErnestaP, @kpedro88, @cmsbuild, @chayanit, @srimanob, @jfernan2, @ahmad3213, @slava77, @jpata, @rvenditti can you please check and sign again.

cmsbuild · 2021-09-06T11:26:55Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33885/25091

This PR adds an extra 36KB to repository
Found files with invalid states:
- CommonTools/ParticleFlow/plugins/PFnoPileUp.cc:
  - Added: e10f0bf
  - Deleted: 556d75d
- CommonTools/ParticleFlow/plugins/PFnoPileUp.h:
  - Added: e10f0bf
  - Deleted: 556d75d
- CommonTools/ParticleFlow/plugins/PFnoPileUpPacked.cc:
  - Added: 6c85589
  - Modified: 5e94e09, ff72b04
  - Deleted: 67c5d8e

cmsbuild · 2021-09-06T11:27:16Z

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

jpata · 2021-09-06T12:43:13Z

@cmsbuild please test

cmsbuild · 2021-09-06T16:22:22Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ae744e/18334/summary.html
COMMIT: d216e83
CMSSW: CMSSW_12_1_X_2021-09-05-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33885/18334/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 4 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ae744e/18334/llvm-analysis/esrget-sa.txt for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 7943 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3001001
DQMHistoTests: Total failures: 6676
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2994302
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 38 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: found differences in 1 / 38 workflows

jpata · 2021-09-09T11:13:09Z

Going through the reco changes, I still see the changes to puppi MET, which I understand is expected: #33885 (comment)

https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_1_X_2021-09-05-2300+ae744e/45242/validateJR/all_OldVSNew_TTbar13nanoEDM106Xv1in2017wf1325p81/all_OldVSNew_TTbar13nanoEDM106Xv1in2017wf1325p81c_nanoaodFlatTable_puppiMetTable__DQM_obj_floats__pt_341.png

There are also slight changes PF jets, isolations and btags, per the presentation at the reco meeting on August 20, I understand this is expected.

https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_1_X_2021-09-05-2300+ae744e/45242/validateJR/all_OldVSNew_TTbar14TeV2023wf12434p0/all_OldVSNew_TTbar14TeV2023wf12434p0c_log10recoPFJets_ak4PFJetsCHS__RECO_obj_et.png

I don't notice any unrelated changes in the reco checks.

@ahinzmann could you please update the PR description with the slide link to the reco meeting ~~("validation presented at PF meeting on June 4th" seems like it didn't happen).~~ Nevermind, I see the presentation was given by Anna.

jpata · 2021-09-13T10:47:09Z

+reconstruction

for d216e83
unifies the vertex association between CHS and Puppi (details in the slides linked in the description)
consequently, small changes expected and observed in CHS and Puppi jets and Puppi MET, b-taggers
no other reco changes observed

cmsbuild · 2021-09-13T10:47:33Z

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)

perrotta · 2021-09-14T06:46:35Z

+1

Small changes in CHS and Puppi jets and Puppi MET, b-taggers are expected, due to the unification of the vertex association code

arizzi · 2021-10-07T12:45:26Z

can you clarify if the association information can be retrieved from MiniAOD and if this is breaking the usage of PackedCandidate format PV related accessors?

arizzi · 2021-10-07T12:52:10Z

CommonTools/RecoAlgos/src/PrimaryVertexAssignment.cc

-    iVertex = vtxIdMinSignif;
+  // protect high pT particles from association to pileup vertices and assign them to the first vertex
+  if ((fPtMaxCharged_ > 0) && (vtxIdMinSignif >= 0) && (track->pt() > fPtMaxCharged_)) {
+    iVertex = 0;


how can we correctly compute the score of the PV for ranking if we protect "iVertex = 0 " ??

in fact why this need to be done here? the pt check / protection can be done in the calling code!

cmsbuild added this to the CMSSW_12_0_X milestone May 28, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels May 28, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels May 28, 2021

cmsbuild mentioned this pull request May 29, 2021

Adding Xi-compatible tracks to lostTracks collection #33891

Merged

cmsbuild added code-checks-pending dqm-pending pdmv-pending upgrade-pending and removed code-checks-approved labels May 29, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels May 29, 2021

ahinzmann force-pushed the primaryVertexAssociationJME120 branch from 4f0c836 to ffeca50 Compare May 29, 2021 21:41

cmsbuild added code-checks-pending and removed code-checks-approved dqm-pending pdmv-pending upgrade-pending labels May 29, 2021

ahinzmann force-pushed the primaryVertexAssociationJME120 branch from ffeca50 to b84be9a Compare May 29, 2021 21:45

cmsbuild added code-checks-approved and removed code-checks-pending labels May 29, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 6, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 6, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 6, 2021

cmsbuild mentioned this pull request Sep 9, 2021

Merge .h and .cc files of plugins in some CommonTools packages #35216

Merged

cmsbuild removed the reconstruction-pending label Sep 13, 2021

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures labels Sep 13, 2021

cmsbuild added orp-approved and removed orp-pending labels Sep 14, 2021

cmsbuild merged commit 2c93e59 into cms-sw:master Sep 14, 2021

cmsbuild mentioned this pull request Sep 15, 2021

Run3-alca200 Bug fix to 2 classes in Calibration/HcalCalibAlgos #35282

Merged

mariadalfonso mentioned this pull request Sep 28, 2021

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

Closed

39 tasks

arizzi reviewed Oct 7, 2021

View reviewed changes

jpata mentioned this pull request Oct 8, 2021

CHS/Puppi vertex unification follow-up #35583

Closed

nurfikri89 mentioned this pull request Oct 12, 2021

[JMENanoAOD] Rework for Prompt #35614

Closed

jpata mentioned this pull request Nov 4, 2021

slowdown in PFCandidatePrimaryVertexSorter in CMSSW_12_1_0_pre2 -> CMSSW_12_1_0_pre3 #35986

Closed

ahinzmann mentioned this pull request Jan 6, 2022

CHS and PUPPI track-vertex-association for Run3 #36639

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify vertex association between CHS and PUPPI #33885

Unify vertex association between CHS and PUPPI #33885

ahinzmann commented May 28, 2021 •

edited

cmsbuild commented May 28, 2021

cmsbuild commented May 28, 2021

slava77 commented May 29, 2021 •

edited

cmsbuild commented May 29, 2021

cmsbuild commented May 29, 2021

cmsbuild commented Sep 6, 2021

cmsbuild commented Sep 6, 2021

jpata commented Sep 6, 2021

cmsbuild commented Sep 6, 2021

jpata commented Sep 9, 2021 •

edited

jpata commented Sep 13, 2021

cmsbuild commented Sep 13, 2021

perrotta commented Sep 14, 2021

arizzi commented Oct 7, 2021

arizzi Oct 7, 2021

arizzi Oct 7, 2021

Unify vertex association between CHS and PUPPI #33885

Unify vertex association between CHS and PUPPI #33885

Conversation

ahinzmann commented May 28, 2021 • edited

PR description:

PR validation:

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

cmsbuild commented May 28, 2021

cmsbuild commented May 28, 2021

slava77 commented May 29, 2021 • edited

cmsbuild commented May 29, 2021

cmsbuild commented May 29, 2021

cmsbuild commented Sep 6, 2021

cmsbuild commented Sep 6, 2021

jpata commented Sep 6, 2021

cmsbuild commented Sep 6, 2021

Comparison Summary

jpata commented Sep 9, 2021 • edited

jpata commented Sep 13, 2021

cmsbuild commented Sep 13, 2021

perrotta commented Sep 14, 2021

arizzi commented Oct 7, 2021

arizzi Oct 7, 2021

Choose a reason for hiding this comment

arizzi Oct 7, 2021

Choose a reason for hiding this comment

ahinzmann commented May 28, 2021 •

edited

slava77 commented May 29, 2021 •

edited

jpata commented Sep 9, 2021 •

edited