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

Convert JetConstituentSelector template from legacy to stream filter #17215

Merged
merged 4 commits into from Jan 27, 2017

Conversation

knoepfel
Copy link
Contributor

No description provided.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @knoepfel (Kyle Knoepfel) for CMSSW_9_0_X.

It involves the following packages:

CommonTools/RecoAlgos

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@rappoccio, @jdolen, @makortel, @abbiendi, @jhgoh, @ahinzmann, @gkasieczka this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@rappoccio
Copy link
Contributor

Shouldn't have any problems, from what I can see.

class JetConstituentSelector : public edm::EDFilter {

template <class T, typename C = std::vector<typename T::ConstituentTypeFwdPtr>>
class JetConstituentSelector : public edm::stream::EDFilter<> {
Copy link
Contributor

Choose a reason for hiding this comment

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

since "filter_ " is removed and the ::filter method return value is true, there is no more excuse to call this EDFilter.
Please change to EDProducer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable, although it requires changing the following configuration files:

RecoJets/Configuration/python/RecoPFJets_cff.py:ak8PFJetsCHSConstituents = cms.EDFilter("PFJetConstituentSelector",
RecoJets/JetProducers/python/ak8PFJetsCS_cfi.py:ak8PFJetsCSConstituents = cms.EDFilter("PFJetConstituentSelector",

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed, some related files will need an update.
Please proceed.

@cmsbuild
Copy link
Contributor

Pull request #17215 was updated. @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Jan 27, 2017

@cmsbuild please test

I must have missed this was updated a few days ago

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 27, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17472/console Started: 2017/01/27 15:32

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Jan 27, 2017

+1

for #17215 c91e20e

  • changes are in line with the description and the follow up review (the selectors are now EDProducers as they were never actually filtering)
  • jenkins tests pass and comparisons with baseline show no difference

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 18a0aee into cms-sw:CMSSW_9_0_X Jan 27, 2017
@knoepfel knoepfel deleted the convert-JetConstituentSelector branch January 30, 2017 14:46
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

5 participants