Puppi #5500

violatingcp · 2014-09-23T09:10:54Z

Puppi's First CMSSW Commit

cmsbuild · 2014-09-23T09:12:51Z

A new Pull Request was created by @violatingcp (Philip Harris) for CMSSW_7_3_X.

Puppi

It involves the following packages:

CommonTools/Puppi

The following packages do not have a category, yet:

CommonTools/Puppi

@cmsbuild, @nclopezo, @ktf can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

ktf · 2014-09-23T09:13:18Z

Out of curiosity, what's Puppi???

violatingcp · 2014-09-23T09:25:33Z

PUPPI = Pile Up Per Particle Identification
(see Page 18 below for details)
http://cms-physics.web.cern.ch/cms-physics/public/JME-14-001-pas.pdf

On Tue, Sep 23, 2014 at 11:13 AM, Giulio Eulisse notifications@github.com
wrote:

Out of curiosity, what's Puppi???

—
Reply to this email directly or view it on GitHub
#5500 (comment).

ktf · 2014-09-23T09:41:55Z

Ok, I guess we do not have much choice for the package name. Subsystem we should probably have something more specific though. @vadler @monttj @slava77 @StoyanStoynev @davidlange6, ideas?

slava77 · 2014-09-23T12:39:28Z

@ktf Uhm, at the moment, the method is simple enough to fit in CommonTools/ParticleFlow
In some remote sense it's a generalization of the PFPileUp.
Phil, if this works for you, please move it to CommonTools/ParticleFlow

After a quick glance, there will be a lot of comments on code compliance.

cmsbuild · 2014-09-24T12:13:24Z

-1
Tested at: b3156a0
I found an error when building:

>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_3_X-slc6_amd64_gcc481/CMSSW_7_3_X_2014-09-23-0200/src/CommonTools/Puppi/plugins/PuppiProducer.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_3_X-slc6_amd64_gcc481/CMSSW_7_3_X_2014-09-23-0200/src/FWCore/Version/src/GetFileFormatVersion.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_3_X-slc6_amd64_gcc481/CMSSW_7_3_X_2014-09-23-0200/src/FWCore/Version/src/GetReleaseVersion.cc 
>> Building shared library tmp/slc6_amd64_gcc481/src/FWCore/Version/src/FWCoreVersion/libFWCoreVersion.so
In file included from /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_3_X-slc6_amd64_gcc481/CMSSW_7_3_X_2014-09-23-0200/src/CommonTools/Puppi/src/PuppiContainer.cc:1:0:
/build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_3_X-slc6_amd64_gcc481/CMSSW_7_3_X_2014-09-23-0200/src/CommonTools/Puppi/interface/PuppiContainer.h:5:50: fatal error: CommonTools/Puppi/interface/NoTrees.hh: No such file or directory
 #include "CommonTools/Puppi/interface/NoTrees.hh"
                                                  ^
compilation terminated.
Copying tmp/slc6_amd64_gcc481/src/FWCore/Version/src/FWCoreVersion/libFWCoreVersion.so to productstore area:
Leaving library rule at FWCore/Version

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5500/59/summary.html

violatingcp · 2014-09-24T16:41:43Z

Hi Slava,

Puppi is kind its own thing. Its not really particle flow. There will be
further developments, like how we treat isolated photons etc...so we would
to keep it separate from the ParticleFlow directory.

Phil

On Tue, Sep 23, 2014 at 2:39 PM, Slava Krutelyov notifications@github.com
wrote:

@ktf https://github.com/ktf Uhm, at the moment, the method is simple
enough to fit in CommonTools/ParticleFlow
In some remote sense it's a generalization of the PFPileUp.
Phil, if this works for you, please move it to CommonTools/ParticleFlow

After a quick glance, there will be a lot of comments on code compliance.

—
Reply to this email directly or view it on GitHub
#5500 (comment).

cmsbuild · 2014-09-24T16:42:45Z

Pull request #5500 was updated. @ktf can you please check and sign again.

slava77 · 2014-09-24T16:51:38Z

Phil,
How is the treatment of photons going to make it unspecific to PF?
Will you still be producing weights per PF candidate or will you be producing weights per reco::Photons?

violatingcp · 2014-09-24T16:54:21Z

We need to remove the photons passing an id and then compute the weights,
the same goes for the leptons. How much we remove and the actual ids that
we use for these are up for debate.

Phil

On Wed, Sep 24, 2014 at 6:51 PM, Slava Krutelyov notifications@github.com
wrote:

Phil,
How is the treatment of photons going to make it unspecific to PF?
Will you still be producing weights per PF candidate or will you be
producing weights per reco::Photons?

—
Reply to this email directly or view it on GitHub
#5500 (comment).

slava77 · 2014-09-24T17:14:04Z

So, as I understand, the purpose/product of the puppi producer will still remain the same: produce a collection of PFCandidates and weights for them.
If so, I still think CommonTools/ParticleFlow is the place.

slava77 · 2014-09-30T13:37:14Z

Phil, in the past we had issues with too many packages,
hence my suggestions here to keep it in a place with code serving similar purpose.

@ktf
Giulio, does the problem of too many packages still exist, what's the guideline from the Core?

If we can easily fragment into separate packages, then I would like to ask to add this to RECO and analysis categories

ktf · 2014-10-01T12:27:27Z

I'd rather not have a new package, unless this introduces extra memory
footprint (which is most likely not the case) or a new external
dependency.

slava77 · 2014-10-01T12:34:21Z

CommonTools/Puppi/interface/PuppiAlgo.h

+  //Get the Weight
+  double compute(std::vector<double> &iVals,double iChi2);
+  //Helpers
+  double ptMin();


this and other methods that don't modify the algo (just simple return value) should be made const
and better be inlined as well

For me to keep track : done.

cmsbuild · 2014-10-27T13:20:57Z

Pull request #5500 was updated. @cmsbuild, @nclopezo can you please check and sign again.

cmsbuild · 2014-10-27T15:56:55Z

+1
Tested at: b85a249
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5500/396/summary.html

cmsbuild · 2014-10-27T16:50:56Z

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

rappoccio · 2014-11-04T16:22:55Z

I started working on this. I am currently implementing above suggestions.

rappoccio · 2014-11-04T16:49:12Z

About the location : CommonTools/ParticleFlow contains all of the "Top Projection" tools. There are a few pileup-specific tools that are coming "on the market" now, and already three of them are in place (Constituent Subtraction, Soft Killer, and Puppi). I would suggest a new package CommonTools/PileupModules to handle these unless there are strong reasons to move it to CommonTools/ParticleFlow. They're really separate things and it would be easier to maintain them separately.

rappoccio · 2014-11-04T18:38:01Z

All code suggestions are now implemented. I will submit a new PR to implement the changes. Please close this one.

Puppi's First Commit

b3156a0

cmsbuild added this to the Next CMSSW_7_3_X milestone Sep 23, 2014

cmsbuild added comparison-pending new-package-pending pending-signatures tests-pending labels Sep 23, 2014

Small fixes to Puppi

1aeb9a5

cmsbuild added comparison-pending new-package-pending pending-signatures tests-rejected and removed comparison-pending new-package-pending pending-signatures tests-pending labels Sep 24, 2014

slava77 reviewed Oct 1, 2014
View reviewed changes

cmsbuild added comparison-pending new-package-pending pending-signatures tests-pending labels Oct 27, 2014

cmsbuild added comparison-pending new-package-pending pending-signatures tests-approved and removed comparison-pending new-package-pending pending-signatures tests-pending labels Oct 27, 2014

cmsbuild added comparison-available new-package-pending pending-signatures tests-approved and removed comparison-pending new-package-pending pending-signatures tests-approved labels Oct 27, 2014

rappoccio mentioned this pull request Nov 7, 2014

Major JME developments for 73x #6273

Closed

davidlange6 closed this Nov 10, 2014

This was referenced Nov 19, 2014

Major JME developments for 73x, Redux #6496

Closed

JME 73x : Add new PU mitigation algos, update to new fastjetcontrib API, Rebase of 6496 #6542

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Puppi #5500

Puppi #5500

violatingcp commented Sep 23, 2014

cmsbuild commented Sep 23, 2014

ktf commented Sep 23, 2014

violatingcp commented Sep 23, 2014

ktf commented Sep 23, 2014

slava77 commented Sep 23, 2014

cmsbuild commented Sep 24, 2014

violatingcp commented Sep 24, 2014

cmsbuild commented Sep 24, 2014

slava77 commented Sep 24, 2014

violatingcp commented Sep 24, 2014

slava77 commented Sep 24, 2014

slava77 commented Sep 30, 2014

ktf commented Oct 1, 2014

slava77 Oct 1, 2014

rappoccio Nov 4, 2014

cmsbuild commented Oct 27, 2014

cmsbuild commented Oct 27, 2014

cmsbuild commented Oct 27, 2014

rappoccio commented Nov 4, 2014

rappoccio commented Nov 4, 2014

rappoccio commented Nov 4, 2014

Puppi #5500

Puppi #5500

Conversation

violatingcp commented Sep 23, 2014

cmsbuild commented Sep 23, 2014

ktf commented Sep 23, 2014

violatingcp commented Sep 23, 2014

ktf commented Sep 23, 2014

slava77 commented Sep 23, 2014

cmsbuild commented Sep 24, 2014

violatingcp commented Sep 24, 2014

cmsbuild commented Sep 24, 2014

slava77 commented Sep 24, 2014

violatingcp commented Sep 24, 2014

slava77 commented Sep 24, 2014

slava77 commented Sep 30, 2014

ktf commented Oct 1, 2014

slava77 Oct 1, 2014

Choose a reason for hiding this comment

rappoccio Nov 4, 2014

Choose a reason for hiding this comment

cmsbuild commented Oct 27, 2014

cmsbuild commented Oct 27, 2014

cmsbuild commented Oct 27, 2014

rappoccio commented Nov 4, 2014

rappoccio commented Nov 4, 2014

rappoccio commented Nov 4, 2014