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

Addition of code for nSubjettiness and QJets #2429

Merged
merged 6 commits into from Mar 5, 2014

Conversation

jstupak
Copy link
Contributor

@jstupak jstupak commented Feb 12, 2014

Yesterday I submitted a pull request (#2414) with the nSubjettiness and QJets code, along with code for gluon tagging and pileup ID. The gluon tagging and pileup ID code needs some work, but (I hope) all the code in this pull request is okay.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @jstupak for CMSSW_7_1_X.

Addition of code for nSubjettiness and QJets

It involves the following packages:

RecoJets/JetAlgorithms
RecoJets/JetProducers

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
@yslai, @TaiSakuma 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

@rappoccio
Copy link
Contributor

Hi, Folks,

What's the status of this PR? Any action items from the JME side?

Cheers,
Sal

@nclopezo nclopezo modified the milestones: CMSSW_7_1_0_pre4, CMSSW_7_1_0_pre3 Feb 24, 2014
@davidlange6
Copy link
Contributor

this PR only has new files (except for a trivial inclusion of a plugin). Any reason not to include it?
@slava77, @anton-a?

@slava77
Copy link
Contributor

slava77 commented Feb 26, 2014

working on it

@@ -0,0 +1,52 @@
#ifndef _QJETS_
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use more unique include guards

@slava77
Copy link
Contributor

slava77 commented Feb 26, 2014

Since the code is not run in regular sequences, only follow up to code review (see comments posted) and compilation is necessary from our side

@slava77
Copy link
Contributor

slava77 commented Feb 28, 2014

-1

based on the code review
There is no need to close this PR, feel free to provide fixes and update your branch jstupak:nSubJetti_qJets to get this PR updated automatically

@jstupak
Copy link
Contributor Author

jstupak commented Mar 1, 2014

All modifications requested by Slava are now in the repo, with one exception; The QJets code still uses system random functions. Is it really necessary to switch to CMS random engines given this will not be run in the standard sequence?

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2014

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

@slava77
Copy link
Contributor

slava77 commented Mar 3, 2014

Hi John,

Please change the random generator setting as well.
Something like the one below should do, I think

 edm::Service<edm::RandomNumberGenerator> rng;
 CLHEP::HepRandomEngine* engine = &rng->getEngine(iEvent.streamID());
 float xFrom0To1 = engine->flat();

@jstupak
Copy link
Contributor Author

jstupak commented Mar 4, 2014

All of Slava's modifications are now implemented

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2014

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

@slava77
Copy link
Contributor

slava77 commented Mar 4, 2014

+1

for #2429 3770d1d
compiles OK. not used anywhere at this point.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2014

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2014

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 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 Mar 5, 2014
RecoJets -- Addition of code for nSubjettiness and QJets
@nclopezo nclopezo merged commit f56267d into cms-sw:CMSSW_7_1_X Mar 5, 2014
@nclopezo nclopezo modified the milestones: CMSSW_7_1_0_pre5, CMSSW_7_1_0_pre4 Mar 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.

None yet

6 participants