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 an L1TCaloJetsMatching Module for HLT #19055

Closed
wants to merge 2 commits into from

Conversation

albertdow
Copy link
Contributor

This module is able to match a collection of CaloJets to L1 jets and then sort the matched CaloJets into collections of either two or three jets, depending on certain Pt and mJJ requirements. This module is required for the development of VBF + Tau HLT paths.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2017

A new Pull Request was created by @albertdow for master.

It involves the following packages:

RecoTauTag/HLTProducers

@Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

const edm::EDGetTokenT<trigger::TriggerFilterObjectWithRefs> jetTrigger_;
const double mPt1_Min;
const double mPt2_Min;
const double mMjj_Min;
Copy link
Contributor

Choose a reason for hiding this comment

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

please consistently follow coding rules, and give all member variable names ending with _ or starting with m_

@@ -0,0 +1,35 @@
#ifndef L1TCaloJetsMatching_H
#define L1TCaloJetsMatching_H
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

#ifndef RecoTauTag_HLTProducers_L1TCaloJetsMatching_h
#define RecoTauTag_HLTProducers_L1TCaloJetsMatching_h


void L1TCaloJetsMatching::produce(edm::StreamID iSId, edm::Event& iEvent, const edm::EventSetup& iES) const
{
std::unique_ptr<reco::CaloJetCollection> caloMatchedJets(new reco::CaloJetCollection);
Copy link
Contributor

Choose a reason for hiding this comment

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

just a suggestion: use

auto caloMatchedJets = std::make_unique<reco::CaloJetCollection>();

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, you do not put this in the Event, so you should simply use a local variable

reco::CaloJetCollection caloMatchedJets;

deltaR2 = ROOT::Math::VectorUtil::DeltaR2(myJet.p4().Vect(), (jetCandRefVec[iL1Jet]->p4()).Vect());
if(deltaR2 < matchingR2 ) {
caloMatchedJets->push_back(myJet);
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

You are looking for any one matching pair, not the closest ?


std::pair<reco::CaloJetCollection,reco::CaloJetCollection> pairJets = categorise(*caloMatchedJets, mPt1_Min, mPt2_Min, mMjj_Min);
std::unique_ptr<reco::CaloJetCollection> twoJets(new reco::CaloJetCollection(pairJets.first));
std::unique_ptr<reco::CaloJetCollection> threeJets(new reco::CaloJetCollection(pairJets.second));
Copy link
Contributor

Choose a reason for hiding this comment

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

auto twoJets = std::make_unique<reco::CaloJetCollection>(pairJets.first);
auto threeJets = std::make_unique<reco::CaloJetCollection>(pairJets.second);

@Martin-Grunewald
Copy link
Contributor

This looks like a duplication of #18992 for a different collection type (Calo vs PF).
In this case we use a single class templated on the collection(s) and instantiate
for each use case.

@Martin-Grunewald
Copy link
Contributor

-1

@albertdow
Copy link
Contributor Author

Moved to #19068

@albertdow albertdow closed this Jun 1, 2017
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

4 participants