PFCluster truth selector #18106

rafaellopesdesa · 2017-03-28T11:17:48Z

This is a small tool that we use at EGM to select ECAL PFcluster matched to GEN photons that did not convert before the calorimeter. The commits are uploaded in EgammaMCTools because of the dependency on GEN (for matching) and SIM (to veto conversions using trackingParticles).

We need it for 90X in the context of the high-level PF studies for the 2017 run, but we are PRing here first (as it is required) and, after review and integration in 91X, we will immediately backport to 90X.

The PR does not alter any standard validation sequence since it is not run by default. It consist of 4 files: the EDProducer, the BuildFile to compile, the python cfi for the producer and a small test script that basically does what this tool is supposed to used to: make ECAL PF Cluster, make trackingParticles, match and select.

Attn: @jainshilpi , @fcouderc

cmsbuild · 2017-03-28T11:18:06Z

A new Pull Request was created by @rafaellopesdesa (Rafael Lopes de Sa) for master.

It involves the following packages:

RecoEgamma/EgammaMCTools

@perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@Sam-Harper, @lgray this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

slava77 · 2017-03-28T13:02:52Z

@cmsbuild please test

cmsbuild · 2017-03-28T13:03:05Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18755/console Started: 2017/03/28 15:05

cmsbuild · 2017-03-28T14:19:22Z

+1
Tested at: bca05dc
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18106/18755/summary.html

cmsbuild · 2017-03-28T14:19:25Z

Comparison job queued.

cmsbuild · 2017-03-28T16:10:39Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18106/18755/summary.html

perrotta · 2017-04-03T12:26:45Z

RecoEgamma/EgammaMCTools/plugins/PFClusterMatchedToPhotonsSelector.cc

+#include "SimDataFormats/TrackingAnalysis/interface/TrackingParticleFwd.h"
+
+
+class PFClusterMatchedToPhotonsSelector : public edm::EDProducer {


Is there any reason for it being a legacy module?
Shouldn't it better inherit from public edm::stream::EDProducer<> ?

perrotta · 2017-04-03T12:28:31Z

RecoEgamma/EgammaMCTools/plugins/PFClusterMatchedToPhotonsSelector.cc

+
+      bool isConversion = false;
+      for (auto&& vertRef : trackingParticle.decayVertices()) {
+	if (vertRef->position().rho() > 123.8 && std::abs(vertRef->position().z()) < 304.5) continue; //EB


Can't these 123.8 and 304.5 be taken from some definition in dataformats or geometry?
Otherwise, define some constant with reasonable name for these boundaries and use it

I can't find an easy way to do this. Can it be done like in the TRK-POG code here:

https://github.com/cms-sw/cmssw/blob/master/SimGeneral/TrackingAnalysis/python/TrackingParticleSelection_cfi.py

ie, defined as a parameter?

yes, a const parameter with a reasonable name is better than a magic number

perrotta · 2017-04-03T12:29:01Z

RecoEgamma/EgammaMCTools/plugins/PFClusterMatchedToPhotonsSelector.cc

+      bool isConversion = false;
+      for (auto&& vertRef : trackingParticle.decayVertices()) {
+	if (vertRef->position().rho() > 123.8 && std::abs(vertRef->position().z()) < 304.5) continue; //EB
+	if (std::abs(vertRef->position().z()) > 317.0) continue; // EE


Can't this 317.0 be taken from some definition in dataformats or geometry?
Otherwise, define some constant with reasonable name for these boundaries and use it

Same comment as above. I will put as a parameter, following the selection done in the tracking POG code.

perrotta · 2017-04-03T12:31:04Z

RecoEgamma/EgammaMCTools/python/pfClusterMatchedToPhotonsSelector_cfi.py

@@ -0,0 +1,10 @@
+import FWCore.ParameterSet.Config as cms
+
+particleFlowClusterECALMatchedToPhotons = cms.EDProducer('PFClusterMatchedToPhotonsSelector',


This duplicates the config automatically produced by ParameterSetDescription
Please remove this file

I will remove the description here and leave the python file, if that's ok with you.

No: please stick on fillDescription and remove this file.
Explicit python config can be used if you want to clone and modify the default module

cmsbuild · 2017-04-07T14:17:20Z

Pull request #18106 was updated. @perrotta, @perrozzi, @thuer, @cmsbuild, @slava77, @govoni, @davidlange6 can you please check and sign again.

perrotta · 2017-04-08T07:50:11Z

please test

cmsbuild · 2017-04-08T07:50:29Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19020/console Started: 2017/04/08 09:51

cmsbuild · 2017-04-08T09:27:46Z

+1
Tested at: 5638c70
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18106/19020/summary.html

cmsbuild · 2017-04-08T09:27:49Z

Comparison job queued.

cmsbuild · 2017-04-08T10:41:33Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18106/19020/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 23
DQMHistoTests: Total histograms compared: 1917345
DQMHistoTests: Total failures: 117
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1917055
DQMHistoTests: Total skipped: 173
DQMHistoTests: Total Missing objects: 0
Checked 94 log files, 14 edm output root files, 23 DQM output files

perrotta · 2017-04-08T12:48:16Z

+1
New producer used in some local test, with no effext on standard workflows
Implementation looks correct

rafaellopesdesa · 2017-04-08T19:04:08Z

@perrozzi, @thuer, @govoni In order to help the generator approval, I would like to describe the single change I did in DataFormats/HepMCCandidate. I added to the dictionaries generated:

edm::ValueMap < reco::GenParticleRef >
edm::Wrapper < edm::ValueMap < reco::GenParticleRef > >

Nothing else was changed.

perrozzi · 2017-04-11T06:40:51Z

+1

cmsbuild · 2017-04-11T06:41:11Z

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 requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar

davidlange6 · 2017-04-11T09:23:31Z

+1

rafaellopesdesa added 2 commits March 28, 2017 01:27

select ecal pfclusters matched to unconverted photons

df2ab5b

makes a cleaner test for the filter

bca05dc

cmsbuild added this to the Next CMSSW_9_1_X milestone Mar 28, 2017

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Mar 28, 2017

cmsbuild added tests-started and removed tests-pending labels Mar 28, 2017

cmsbuild added tests-approved and removed tests-started labels Mar 28, 2017

cmsbuild added comparison-available and removed comparison-pending labels Mar 28, 2017

perrotta reviewed Apr 3, 2017

View reviewed changes

rafaellopesdesa mentioned this pull request Apr 5, 2017

PFCluster studies tree cms-egamma/RegressionTreeProducer#3

Closed

rafaellopesdesa added 2 commits April 5, 2017 15:27

dictionary for genparticles ref valuemap

464b0ea

code review and add auxiliary products for PS and GEN matching

6ac516f

cmsbuild added comparison-pending and removed comparison-available tests-approved labels Apr 5, 2017

cmsbuild added comparison-pending generators-pending tests-pending labels Apr 7, 2017

cmsbuild added tests-started and removed tests-pending labels Apr 8, 2017

cmsbuild added tests-approved and removed tests-started labels Apr 8, 2017

cmsbuild added comparison-available and removed comparison-pending labels Apr 8, 2017

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Apr 8, 2017

rafaellopesdesa mentioned this pull request Apr 10, 2017

Pf cluster truth selector 90X #18285

Closed

cmsbuild added fully-signed generators-approved and removed generators-pending pending-signatures labels Apr 11, 2017

cmsbuild added orp-approved and removed orp-pending labels Apr 11, 2017

cmsbuild merged commit 1a07832 into cms-sw:master Apr 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PFCluster truth selector #18106

PFCluster truth selector #18106

rafaellopesdesa commented Mar 28, 2017

cmsbuild commented Mar 28, 2017

slava77 commented Mar 28, 2017

cmsbuild commented Mar 28, 2017 •

edited

cmsbuild commented Mar 28, 2017

cmsbuild commented Mar 28, 2017

cmsbuild commented Mar 28, 2017

perrotta Apr 3, 2017

perrotta Apr 3, 2017

rafaellopesdesa Apr 5, 2017

perrotta Apr 5, 2017

perrotta Apr 3, 2017

rafaellopesdesa Apr 5, 2017

perrotta Apr 3, 2017

rafaellopesdesa Apr 5, 2017

perrotta Apr 5, 2017

cmsbuild commented Apr 7, 2017

perrotta commented Apr 8, 2017

cmsbuild commented Apr 8, 2017 •

edited

cmsbuild commented Apr 8, 2017

cmsbuild commented Apr 8, 2017

cmsbuild commented Apr 8, 2017

perrotta commented Apr 8, 2017

rafaellopesdesa commented Apr 8, 2017

perrozzi commented Apr 11, 2017

cmsbuild commented Apr 11, 2017

davidlange6 commented Apr 11, 2017

		#include "SimDataFormats/TrackingAnalysis/interface/TrackingParticleFwd.h"


		class PFClusterMatchedToPhotonsSelector : public edm::EDProducer {

		@@ -0,0 +1,10 @@
		import FWCore.ParameterSet.Config as cms

		particleFlowClusterECALMatchedToPhotons = cms.EDProducer('PFClusterMatchedToPhotonsSelector',

PFCluster truth selector #18106

PFCluster truth selector #18106

Conversation

rafaellopesdesa commented Mar 28, 2017

cmsbuild commented Mar 28, 2017

slava77 commented Mar 28, 2017

cmsbuild commented Mar 28, 2017 • edited

cmsbuild commented Mar 28, 2017

cmsbuild commented Mar 28, 2017

cmsbuild commented Mar 28, 2017

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

cmsbuild commented Apr 7, 2017

perrotta commented Apr 8, 2017

cmsbuild commented Apr 8, 2017 • edited

cmsbuild commented Apr 8, 2017

cmsbuild commented Apr 8, 2017

cmsbuild commented Apr 8, 2017

perrotta commented Apr 8, 2017

rafaellopesdesa commented Apr 8, 2017

perrozzi commented Apr 11, 2017

cmsbuild commented Apr 11, 2017

davidlange6 commented Apr 11, 2017

cmsbuild commented Mar 28, 2017 •

edited

cmsbuild commented Apr 8, 2017 •

edited