Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding converter for Basic to PF Jets #3220

Merged

Conversation

clint-richardson
Copy link
Contributor

As the title suggests this is to add a plugin that will convert reco::BasicJets to reco::PFJets. Such a converter is needed in order to work with Basic Jets using HLTFilters.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2014

A new Pull Request was created by @clint-richardson (Clint Richardson) for CMSSW_7_1_X.

adding converter for Basic to PF Jets

It involves the following packages:

RecoJets/JetProducers

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
@yslai, @TaiSakuma, @nhanvtran this is something you requested to watch as well.
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.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2014

-1
I found an error when building:

>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-04-08-0200/src/RecoJets/JetProducers/src/PileupJetIdAlgo.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-04-08-0200/src/RecoJets/JetProducers/src/VirtualJetProducerHelper.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-04-08-0200/src/RecoJets/JetProducers/test/test-large-voronoi-area.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-04-08-0200/src/RecoJets/JetProducers/plugins/BasicToPFJet.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-04-08-0200/src/RecoJets/JetProducers/plugins/BoostedTauSeedsProducer.cc 
/build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-04-08-0200/src/RecoJets/JetProducers/plugins/BasicToPFJet.cc:21:39: fatal error: ../interface/BasicToPFJet.h: No such file or directory
 #include "../interface/BasicToPFJet.h"
                                       ^
compilation terminated.
/build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-04-08-0200/src/RecoJets/JetProducers/plugins/BasicToPFJet.cc:21:39: fatal error: ../interface/BasicToPFJet.h: No such file or directory
 #include "../interface/BasicToPFJet.h"


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

//
// Original Author: clint richardson
// Created: Thu, 6 Mar 2014 12:00:00 GMT
// $Id$
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove "$Id$", CVS-related special

@slava77
Copy link
Contributor

slava77 commented Apr 8, 2014

-1

see comments from the code review

@clint-richardson
Copy link
Contributor Author

Hi @slava77

I've attempted to comply with your requests. Can you let me know what else I'm missing? I didn't understand the comment that was 'why not start here from the beginning' - could you explain it to me? Thanks,
-Clint

@clint-richardson
Copy link
Contributor Author

Hi @slava77

I've made the change

Best,
-Clint

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2014

Pull request #3220 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2014

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2014

@slava77
Copy link
Contributor

slava77 commented Apr 9, 2014

+1

for #3220 8fa4ca6

tested in
CMSSW_7_1_X_2014-04-08-0200 (test area sign338) in a combination of pulls 3233 and 3220.
No changes observed, none expected.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

nclopezo added a commit that referenced this pull request Apr 10, 2014
…tConverter

RecoJets/JetProducers -- adding converter for Basic to PF Jets
@nclopezo nclopezo merged commit 67bf54a into cms-sw:CMSSW_7_1_X Apr 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants