Track-gen matching passing through TrackingParticle #33774

elusian · 2021-05-19T10:03:40Z

PR description:

This PR adds a way to match generalTracks (in AOD) and packedPFCandidates and lostTracks (in miniAOD) to a subset of genParticles, using association by hit on TrackingParticles.
The association is performed on step3 and just replicated in the miniAOD.
Since the track association by hit needs both TrackingParticles and SimLink for pixel and strips and the SimLink collections are not available in RAWSIM, two plugins are created to be run in step2 to add two pruned collections and avoid adding the all the SimLink, which have a non negligible size.

The changes contained are:

a plugin for pruning TrackingParticles in step2 based on their association to specific GenParticles. The genParticle selection is taken from the GenParticlePruner class and can be configured.
a plugin for pruning SimLink collections based on the SimTrack association to a TrackingParticle
a plugin for adding an association between PackedCandidates and GenParticles based on an existing association between tracks and GenParticles
Needed configuration changes to add this to standard sequences

No plugin is needed for association between tracks and GenParticles through TrackingParticle as it is already present in CMSSW (MCTrackMatcher).

PR validation:

On a B+->Psi2SK sample (one of the possible targets for this PR) we registered this increases in size, timing and memory

	Size	Timing	Memory
RAWSIM	+0.05%	+0.08%	+5MB
RECOSIM	< +0.01%	+0.07%	+2MB
AODSIM	+0.06%
MINIAODSIM	+0.4%	+0.08%	+400kB

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

Not a backport, but if possible we would like to backport this to 10_6 for MC production for BParking reprocessing

cmsbuild · 2021-05-19T10:10:07Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33774/22739

This PR adds an extra 16KB to repository

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

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33774/22739/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33774/22739/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-05-20T11:46:30Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33774/22772

This PR adds an extra 16KB to repository

cmsbuild · 2021-05-20T11:46:55Z

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

It involves the following packages:

SimTracker/TrackAssociation

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@makortel, @mtosi, @abbiendi, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @jhgoh, @threus, @dgulhan 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

mariadalfonso · 2021-05-20T11:50:47Z

please test

cmsbuild · 2021-05-20T14:37:11Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dfca37/15210/summary.html
COMMIT: cfd802f
CMSSW: CMSSW_12_0_X_2021-05-19-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33774/15210/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2648902
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2648873
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

makortel · 2021-05-20T14:50:11Z

SimTracker/TrackAssociation/plugins/PackedCandidateGenAssociationProducer.cc

+// class declaration
+//
+
+class PackedCandidateGenAssociationProducer : public edm::stream::EDProducer<> {


Looks like this could be edm::global module.

makortel · 2021-05-20T14:50:41Z

SimTracker/TrackAssociation/plugins/SimLinkPruner.cc

+// class declaration
+//
+
+class SimLinkPruner : public edm::stream::EDProducer<> {


Looks like this too could be edm::global module.

makortel · 2021-05-20T15:04:27Z

SimTracker/TrackAssociation/plugins/TrackingParticleSelectorByGen.cc

+  using namespace edm;
+
+  if (firstEvent_) {
+    PdgEntryReplacer rep(iSetup);


This needs to be migrated to EventSetup consumes. (I understand it would have implications outside of the code added in this PR, so this may be a note more for @cms-sw/simulation-l2)

mmusich

Hi,
I think a lot of the event data access boilerplate can be simplified to improve readability as you don't check the validity of the handles, as suggested at: https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideEDMGetDataFromEvent#get.
Also I would suggest a better naming for SimLinkPruner -> DigiSimLinkPruner

mmusich · 2021-05-20T14:33:41Z

SimTracker/TrackAssociation/plugins/PackedCandidateGenAssociationProducer.cc

+  edm::Handle<edm::Association<reco::GenParticleCollection>> htrackToGenAssoc;
+  iEvent.getByToken(trackToGenToken_, htrackToGenAssoc);
+  const auto& trackToGenAssoc = *htrackToGenAssoc;


Suggested change

edm::Handle<edm::Association<reco::GenParticleCollection>> htrackToGenAssoc;

iEvent.getByToken(trackToGenToken_, htrackToGenAssoc);

const auto& trackToGenAssoc = *htrackToGenAssoc;

const auto& trackToGenAssoc = iEvent.get(trackToGenToken_);

mmusich · 2021-05-20T14:36:12Z

SimTracker/TrackAssociation/plugins/PackedCandidateGenAssociationProducer.cc

+  edm::Handle<edm::Association<pat::PackedCandidateCollection>> htrackToPackedCandidatesAssoc;
+  iEvent.getByToken(trackToPcToken_, htrackToPackedCandidatesAssoc);
+  const auto& trackToPackedCandidatesAssoc = *htrackToPackedCandidatesAssoc;


Suggested change

edm::Handle<edm::Association<pat::PackedCandidateCollection>> htrackToPackedCandidatesAssoc;

iEvent.getByToken(trackToPcToken_, htrackToPackedCandidatesAssoc);

const auto& trackToPackedCandidatesAssoc = *htrackToPackedCandidatesAssoc;

const auto& trackToPackedCandidatesAssoc = iEvent.get(trackToPcToken_);

mmusich · 2021-05-20T14:36:41Z

SimTracker/TrackAssociation/plugins/PackedCandidateGenAssociationProducer.cc

+  edm::Handle<edm::Association<reco::GenParticleCollection>> hgenToPrunedAssoc;
+  iEvent.getByToken(genToPrunedToken_, hgenToPrunedAssoc);
+  const auto& genToPrunedAssoc = *hgenToPrunedAssoc;


Suggested change

edm::Handle<edm::Association<reco::GenParticleCollection>> hgenToPrunedAssoc;

iEvent.getByToken(genToPrunedToken_, hgenToPrunedAssoc);

const auto& genToPrunedAssoc = *hgenToPrunedAssoc;

const auto& genToPrunedAssoc = iEvent.get(genToPrunedToken_);

mmusich · 2021-05-20T14:37:15Z

SimTracker/TrackAssociation/plugins/PackedCandidateGenAssociationProducer.cc

+  edm::Handle<edm::Association<reco::GenParticleCollection>> hgenToPrunedAssocWSO;
+  iEvent.getByToken(genToPrunedWSOToken_, hgenToPrunedAssocWSO);
+  const auto& genToPrunedAssocWSO = *hgenToPrunedAssocWSO;


Suggested change

edm::Handle<edm::Association<reco::GenParticleCollection>> hgenToPrunedAssocWSO;

iEvent.getByToken(genToPrunedWSOToken_, hgenToPrunedAssocWSO);

const auto& genToPrunedAssocWSO = *hgenToPrunedAssocWSO;

const auto& genToPrunedAssocWSO = iEvent.get(genToPrunedWSOToken_);

mmusich · 2021-05-20T14:39:33Z

SimTracker/TrackAssociation/plugins/PackedCandidateGenAssociationProducer.cc

+  edm::Handle<edm::View<reco::Track>> htracks;
+  iEvent.getByToken(tracksToken_, htracks);
+  const auto& tracks = *htracks;


Suggested change

edm::Handle<edm::View<reco::Track>> htracks;

iEvent.getByToken(tracksToken_, htracks);

const auto& tracks = *htracks;

const auto& tracks = iEvent.get(tracksToken_);

mmusich · 2021-05-20T14:43:59Z

SimTracker/TrackAssociation/plugins/TrackingParticleSelectorByGen.cc

+    firstEvent_ = false;
+  }
+
+  edm::Handle<TrackingParticleCollection> tps;


as above, but changing the loop at line 315

mmusich · 2021-05-20T14:47:30Z

SimTracker/TrackAssociation/plugins/TrackingParticleSelectorByGen.cc

+  iEvent.getByToken(tpToken_, tps);
+
+  edm::Handle<reco::GenParticleCollection> gps;
+  iEvent.getByToken(gpToken_, gps);


as above, but changing the calls below.

mmusich · 2021-05-20T14:49:55Z

SimTracker/TrackAssociation/plugins/SimLinkPruner.cc

+// ------------ method fills 'descriptions' with the allowed parameters for the module  ------------
+void SimLinkPruner::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
+  edm::ParameterSetDescription desc;
+  desc.add<edm::InputTag>("trackingParticles", edm::InputTag("mix", "MergedTrackTruth"));


how does premixing deal with this?
Or is this meant for Signal TPs only by construction?

mmusich · 2021-05-20T14:50:21Z

SimTracker/TrackAssociation/plugins/TrackingParticleSelectorByGen.cc

+
+void TrackingParticleSelectorByGen::fillDescriptions(edm::ConfigurationDescriptions &descriptions) {
+  edm::ParameterSetDescription desc;
+  desc.add<edm::InputTag>("trackingParticles", edm::InputTag("mix", "MergedTrackTruth"));


how is premixing going to be dealt with?
Or is this meant for Signal TPs only by construction?

mmusich · 2021-05-20T14:50:56Z

SimTracker/TrackAssociation/plugins/SimLinkPruner.cc

+// class declaration
+//
+
+class SimLinkPruner : public edm::stream::EDProducer<> {


I think something like DigiSimLinkPruner would be more appropriate.

civanch · 2021-05-20T17:07:21Z

@elusian , what is the value of extra memory, which bring this PR?

…d on their association to a gen particle

qliphy · 2021-06-12T08:08:55Z

+operations

qliphy · 2021-06-12T08:09:31Z

@cms-sw/pdmv-l2 @cms-sw/upgrade-l2 Do you have further comments?

srimanob · 2021-06-12T13:18:15Z

@elusian

Sorry, I somehow miss this. Could you please clarify few things? (It is maybe somewhere in the long discussion of this PR)

Is this PR support to work properly with premixing? Test on that? I see workflow .181 in PR test.
What is the PU description that you use to measure size addition? Do we have number for PU200? Since the additional products are on RAW without modifier, I assume it will propagate to Phase-2 also.

Thanks and sorry for my late.

slava77 · 2021-06-12T16:48:25Z

@elusian

Sorry, I somehow miss this. Could you please clarify few things? (It is maybe somewhere in the long discussion of this PR)
* Is this PR support to work properly with premixing? Test on that? I see workflow .181 in PR test.

* What is the PU description that you use to measure size addition? Do we have number for PU200? Since the additional products are on RAW without modifier, I assume it will propagate to Phase-2 also.
Thanks and sorry for my late.

as described in #33774 (comment) (even though it was done in 10_6_X), the code works with premixing. Note though that the code is making associations only to the signal tracking particles. So, the presence of pileup doesn't really matter that much.

srimanob · 2021-06-13T01:41:03Z

+Upgrade

Thanks @slava77 for confirmation. That is what I understand from the code too that it will work with signal only for premixing. For Phase-2 size, I think the review can be done later, considering the need of PR for UL now.

chayanit · 2021-06-13T02:34:27Z

+1

cmsbuild · 2021-06-13T02:34:51Z

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

qliphy · 2021-06-13T02:51:40Z

+1

chayanit · 2021-06-28T07:40:05Z

Hello @elusian , just to inform you that because of this PR, we are not able to produce RECO-only RelVal for 12_0_0_pre3 campaign (both Run3 and Phase2). The production failed to find the module and complained product not found. Please see what we have tried here. https://dmytro.web.cern.ch/dmytro/cmsprodmon/workflows.php?campaign=CMSSW_12_0_0_pre3__fullsim_PU_2021_14TeV_RECOonly-1624276545

Fatal Exception (Exit code: 8006)
An exception of category 'ProductNotFound' occurred while
[0] Processing Event run: 1 lumi: 1 event: 2 stream: 0
[1] Running path 'RECOSIMoutput_step'
[2] Prefetching for module PoolOutputModule/'RECOSIMoutput'
[3] Prefetching for module MCTrackMatcher/'prunedTrackMCMatch'
[4] Prefetching for module QuickTrackAssociatorByHitsProducer/'quickPrunedTrackAssociatorByHits'
[5] Calling method for module ClusterTPAssociationProducer/'prunedTpClusterProducer'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vector
Looking for module label: prunedTrackingParticles
Looking for productInstanceName:

srimanob · 2021-06-28T08:23:13Z

@chayanit
As mentioned in the PR description, there is a change in RAW content, two pruned collections are added to step-2 (DIGI step). So, it is not possible to do reco-only.

The change of RAW content is in
SimTracker/Configuration/python/SimTracker_EventContent_cff.py

elusian · 2021-06-28T08:35:14Z

That is correct, the missing product in the RAWSIM was not available on CMSSW_12_0_0_pre2, so it is not possible to run only on the old RAW without configuration changes

chayanit · 2021-06-28T09:04:54Z

Thanks for the confirmation! @srimanob
@elusian let me know if you find some difficulties to validate the campaign and if any other helps we could provide instead of RECO-only samples.

cmsbuild added this to the CMSSW_12_0_X milestone May 19, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures simulation-pending tests-pending labels May 19, 2021

cmsbuild added code-checks-rejected code-checks-pending and removed code-checks-pending code-checks-rejected labels May 19, 2021

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

cmsbuild added tests-started and removed tests-pending labels May 20, 2021

cmsbuild added tests-approved and removed tests-started labels May 20, 2021

makortel reviewed May 20, 2021

View reviewed changes

mmusich reviewed May 20, 2021

View reviewed changes

Enrico Lusiani added 4 commits June 1, 2021 14:26

Add plugin to prune the tracking particle and simlink collection base…

de233a8

…d on their association to a gen particle

Add plugin to propagate a track-gen association from AOD to miniAOD

eb22f87

code format

3657544

apply suggestions

f825701

slava77 mentioned this pull request Jun 1, 2021

features for bParking UL rereco and miniAOD in 10_6_X (10_6_26?) #33917

Closed

14 tasks

add configuration changes

7669be9

cmsbuild added the simulation-approved label Jun 11, 2021

cmsbuild added operations-approved and removed operations-pending labels Jun 12, 2021

cmsbuild added upgrade-approved and removed upgrade-pending labels Jun 13, 2021

cmsbuild added fully-signed pdmv-approved and removed pdmv-pending pending-signatures labels Jun 13, 2021

cmsbuild added orp-approved and removed orp-pending labels Jun 13, 2021

cmsbuild merged commit 05a259b into cms-sw:master Jun 13, 2021

qliphy mentioned this pull request Jun 14, 2021

Unit Failure in 12_X IB from TauAnalysis/MCEmbeddingTools #34112

Closed

elusian mentioned this pull request Jun 14, 2021

Remove the packedCandidate to gen associations in miniAOD_customizeAllData #34113

Merged

elusian mentioned this pull request Jul 21, 2021

[Backport] Track-gen matching passing through TrackingParticle (11_2_X) #34574

Merged

srimanob mentioned this pull request Aug 30, 2021

Add Digi to the modified steps for bParking workflows #35061

Merged

elusian mentioned this pull request Nov 5, 2021

Add a configuration function to disable track-gen matching in step3 from cmsDriver #36011

Merged

mmusich mentioned this pull request Nov 15, 2021

[12.0.X] add a configuration function to disable track-gen matching from cmsDriver #36127

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track-gen matching passing through TrackingParticle #33774

Track-gen matching passing through TrackingParticle #33774

elusian commented May 19, 2021 •

edited

cmsbuild commented May 19, 2021

cmsbuild commented May 20, 2021

cmsbuild commented May 20, 2021

mariadalfonso commented May 20, 2021

cmsbuild commented May 20, 2021

makortel May 20, 2021

makortel May 20, 2021

makortel May 20, 2021 •

edited

mmusich left a comment

mmusich May 20, 2021

mmusich May 20, 2021

mmusich May 20, 2021

mmusich May 20, 2021

mmusich May 20, 2021

mmusich May 20, 2021

mmusich May 20, 2021

mmusich May 20, 2021

mmusich May 20, 2021

mmusich May 20, 2021

civanch commented May 20, 2021

qliphy commented Jun 12, 2021

qliphy commented Jun 12, 2021

srimanob commented Jun 12, 2021

slava77 commented Jun 12, 2021

srimanob commented Jun 13, 2021

chayanit commented Jun 13, 2021

cmsbuild commented Jun 13, 2021

qliphy commented Jun 13, 2021

chayanit commented Jun 28, 2021

srimanob commented Jun 28, 2021

elusian commented Jun 28, 2021

chayanit commented Jun 28, 2021 •

edited

Track-gen matching passing through TrackingParticle #33774

Track-gen matching passing through TrackingParticle #33774

Conversation

elusian commented May 19, 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 19, 2021

cmsbuild commented May 20, 2021

cmsbuild commented May 20, 2021

mariadalfonso commented May 20, 2021

cmsbuild commented May 20, 2021

Comparison Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

makortel May 20, 2021 • edited

Choose a reason for hiding this comment

mmusich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

civanch commented May 20, 2021

qliphy commented Jun 12, 2021

qliphy commented Jun 12, 2021

srimanob commented Jun 12, 2021

slava77 commented Jun 12, 2021

srimanob commented Jun 13, 2021

chayanit commented Jun 13, 2021

cmsbuild commented Jun 13, 2021

qliphy commented Jun 13, 2021

chayanit commented Jun 28, 2021

srimanob commented Jun 28, 2021

elusian commented Jun 28, 2021

chayanit commented Jun 28, 2021 • edited

elusian commented May 19, 2021 •

edited

makortel May 20, 2021 •

edited

chayanit commented Jun 28, 2021 •

edited