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

Reduce non necessary sqrt computations in RecoTauTag/HLTProducers #41100

Merged
merged 4 commits into from Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 3 additions & 11 deletions RecoTauTag/HLTProducers/interface/PFJetsMaxInvMassModule.h
Expand Up @@ -5,17 +5,9 @@
#include "FWCore/Framework/interface/Frameworkfwd.h"
#include "FWCore/Framework/interface/global/EDProducer.h"
#include "FWCore/ParameterSet/interface/ConfigurationDescriptions.h"
#include "FWCore/ParameterSet/interface/ParameterSetDescription.h"
#include "FWCore/Framework/interface/Event.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "FWCore/Utilities/interface/InputTag.h"
#include "DataFormats/Common/interface/Handle.h"
#include "DataFormats/L1Trigger/interface/Tau.h"
#include "DataFormats/JetReco/interface/CaloJetCollection.h"
#include "DataFormats/TauReco/interface/PFTauFwd.h"
#include "DataFormats/HLTReco/interface/TriggerFilterObjectWithRefs.h"
#include "DataFormats/HLTReco/interface/TriggerObject.h"
#include "DataFormats/HLTReco/interface/TriggerEvent.h"
#include "FWCore/Framework/interface/EventSetup.h"
#include "DataFormats/JetReco/interface/PFJetCollection.h"

class PFJetsMaxInvMassModule : public edm::global::EDProducer<> {
private:
Expand All @@ -25,7 +17,7 @@ class PFJetsMaxInvMassModule : public edm::global::EDProducer<> {

public:
explicit PFJetsMaxInvMassModule(const edm::ParameterSet&);
~PFJetsMaxInvMassModule() override;
~PFJetsMaxInvMassModule() override = default;
void produce(edm::StreamID, edm::Event&, const edm::EventSetup&) const override;
static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);
};
Expand Down
4 changes: 2 additions & 2 deletions RecoTauTag/HLTProducers/src/CaloTowerCreatorForTauHLT.cc
Expand Up @@ -57,9 +57,9 @@ void CaloTowerCreatorForTauHLT::produce(StreamID sid, Event& evt, const EventSet
}
if (cal->et() >= mEtThreshold && cal->energy() >= mEThreshold) {
math::PtEtaPhiELorentzVector p(cal->et(), cal->eta(), cal->phi(), cal->energy());
double delta = ROOT::Math::VectorUtil::DeltaR((*myL1Jet).p4().Vect(), p);
double delta2 = ROOT::Math::VectorUtil::DeltaR2((*myL1Jet).p4().Vect(), p);

if (delta < mCone) {
if (delta2 < mCone * mCone) {
Copy link
Contributor

Choose a reason for hiding this comment

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

mCone * mCone could be computed outside the loop. (or in the ctor)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Compiler should be able to optimize it anyhow... But ok, why not? I will update

isAccepted = true;
cands->push_back(*cal);
}
Expand Down
Expand Up @@ -57,8 +57,8 @@ void CaloTowerFromL1TCreatorForTauHLT::produce(StreamID sid, Event& evt, const E
}
if (cal->et() >= mEtThreshold && cal->energy() >= mEThreshold) {
math::PtEtaPhiELorentzVector p(cal->et(), cal->eta(), cal->phi(), cal->energy());
double delta = ROOT::Math::VectorUtil::DeltaR((*myL1Jet).p4().Vect(), p);
if (delta < mCone) {
double delta2 = ROOT::Math::VectorUtil::DeltaR2((*myL1Jet).p4().Vect(), p);
if (delta2 < mCone * mCone) {
Copy link
Contributor

Choose a reason for hiding this comment

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

mCone * mCone could be computed outside the loop. (or in the ctor)

isAccepted = true;
cands->push_back(*cal);
}
Expand Down
7 changes: 4 additions & 3 deletions RecoTauTag/HLTProducers/src/HLTPFDiJetCorrCheckerWithDiTau.cc
Expand Up @@ -15,6 +15,7 @@ The module stores j1, j2 of any (j1, j2, t1, t2) that satisfies the conditions a
#include "FWCore/ParameterSet/interface/ConfigurationDescriptions.h"
#include "FWCore/ParameterSet/interface/ParameterSetDescription.h"
#include "FWCore/Framework/interface/Event.h"
#include "FWCore/Framework/interface/EventSetup.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "FWCore/Utilities/interface/InputTag.h"
#include "FWCore/Utilities/interface/Exception.h"
Expand All @@ -37,7 +38,7 @@ class HLTPFDiJetCorrCheckerWithDiTau : public edm::global::EDProducer<> {
const edm::EDGetTokenT<trigger::TriggerFilterObjectWithRefs> tauSrc_;
const edm::EDGetTokenT<reco::PFJetCollection> pfJetSrc_;
const double extraTauPtCut_;
const double mjjMin_;
const double m2jjMin_;
const double dRmin_, dRmin2_;
// pt comparator
GreaterByPt<reco::PFJet> pTComparator_;
Expand All @@ -50,7 +51,7 @@ HLTPFDiJetCorrCheckerWithDiTau::HLTPFDiJetCorrCheckerWithDiTau(const edm::Parame
: tauSrc_(consumes(iConfig.getParameter<edm::InputTag>("tauSrc"))),
pfJetSrc_(consumes(iConfig.getParameter<edm::InputTag>("pfJetSrc"))),
extraTauPtCut_(iConfig.getParameter<double>("extraTauPtCut")),
mjjMin_(iConfig.getParameter<double>("mjjMin")),
m2jjMin_(iConfig.getParameter<double>("mjjMin") * iConfig.getParameter<double>("mjjMin")),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be improved.

A user might put mjjMin = -1 in the PSet (with the intention of switching off the cut), and the plugin will 'silently' require m2 > 1.

One way to improve this is

m2jjMin_(std::max(0, iConfig.getParameter<double>("mjjMin")) * std::max(0, iConfig.getParameter<double>("mjjMin"))),

The same argument probably applies to other plugins touched by this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct @missirol
The last commit should take care of it

dRmin_(iConfig.getParameter<double>("dRmin")),
dRmin2_(dRmin_ * dRmin_) {
if (dRmin_ <= 0.) {
Expand All @@ -77,7 +78,7 @@ void HLTPFDiJetCorrCheckerWithDiTau::produce(edm::StreamID iSId, edm::Event& iEv
const reco::PFJet& myPFJet1 = pfJets[iJet1];
const reco::PFJet& myPFJet2 = pfJets[iJet2];

if ((myPFJet1.p4() + myPFJet2.p4()).M() < mjjMin_)
if ((myPFJet1.p4() + myPFJet2.p4()).M2() < m2jjMin_)
continue;

for (unsigned int iTau1 = 0; iTau1 < taus.size(); iTau1++) {
Expand Down
11 changes: 5 additions & 6 deletions RecoTauTag/HLTProducers/src/L1HLTJetsMatching.cc
Expand Up @@ -33,8 +33,7 @@ void L1HLTJetsMatching::produce(edm::StreamID, edm::Event& iEvent, const edm::Ev

unique_ptr<CaloJetCollection> tauL2jets(new CaloJetCollection);

double deltaR = 1.0;
double matchingR = 0.5;
constexpr double matchingR = 0.5 * 0.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
constexpr double matchingR = 0.5 * 0.5;
constexpr double matchingR2 = 0.5 * 0.5;

(Clearer name.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a matter of taste... but ok

//Getting HLT jets to be matched
edm::Handle<edm::View<Candidate> > tauJets;
iEvent.getByToken(jetSrc, tauJets);
Expand All @@ -56,8 +55,8 @@ void L1HLTJetsMatching::produce(edm::StreamID, edm::Event& iEvent, const edm::Ev
for (unsigned int iJet = 0; iJet < tauJets->size(); iJet++) {
//Find the relative L2TauJets, to see if it has been reconstructed
const Candidate& myJet = (*tauJets)[iJet];
deltaR = ROOT::Math::VectorUtil::DeltaR(myJet.p4().Vect(), (tauCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR < matchingR) {
double deltaR2 = ROOT::Math::VectorUtil::DeltaR2(myJet.p4().Vect(), (tauCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR2 < matchingR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (deltaR2 < matchingR) {
if (deltaR2 < matchingR2) {

// LeafCandidate myLC(myJet);
CaloJet myCaloJet(myJet.p4(), a, f);
if (myJet.pt() > mEt_Min) {
Expand All @@ -73,8 +72,8 @@ void L1HLTJetsMatching::produce(edm::StreamID, edm::Event& iEvent, const edm::Ev
for (unsigned int iJet = 0; iJet < tauJets->size(); iJet++) {
const Candidate& myJet = (*tauJets)[iJet];
//Find the relative L2TauJets, to see if it has been reconstructed
deltaR = ROOT::Math::VectorUtil::DeltaR(myJet.p4().Vect(), (jetCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR < matchingR) {
double deltaR2 = ROOT::Math::VectorUtil::DeltaR2(myJet.p4().Vect(), (jetCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR2 < matchingR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (deltaR2 < matchingR) {
if (deltaR2 < matchingR2) {

// LeafCandidate myLC(myJet);
CaloJet myCaloJet(myJet.p4(), a, f);
if (myJet.pt() > mEt_Min) {
Expand Down
12 changes: 6 additions & 6 deletions RecoTauTag/HLTProducers/src/L1HLTTauMatching.cc
Expand Up @@ -31,8 +31,8 @@ void L1HLTTauMatching::produce(edm::StreamID iSId, edm::Event& iEvent, const edm

unique_ptr<PFTauCollection> tauL2jets(new PFTauCollection);

double deltaR = 1.0;
double matchingR = 0.5;
constexpr double matchingR = 0.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
constexpr double matchingR = 0.5;
constexpr double matchingR2 = 0.5;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here there was another mistake of mine:

Suggested change
constexpr double matchingR = 0.5;
constexpr double matchingR2 = 0.5 * 0.5;


//Getting HLT jets to be matched
edm::Handle<PFTauCollection> tauJets;
iEvent.getByToken(jetSrc, tauJets);
Expand All @@ -53,8 +53,8 @@ void L1HLTTauMatching::produce(edm::StreamID iSId, edm::Event& iEvent, const edm
for (unsigned int iJet = 0; iJet < tauJets->size(); iJet++) {
//Find the relative L2TauJets, to see if it has been reconstructed
const PFTau& myJet = (*tauJets)[iJet];
deltaR = ROOT::Math::VectorUtil::DeltaR(myJet.p4().Vect(), (tauCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR < matchingR) {
double deltaR2 = ROOT::Math::VectorUtil::DeltaR2(myJet.p4().Vect(), (tauCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR2 < matchingR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (deltaR2 < matchingR) {
if (deltaR2 < matchingR2) {

// LeafCandidate myLC(myJet);
if (myJet.leadChargedHadrCand().isNonnull()) {
a = myJet.leadChargedHadrCand()->vertex();
Expand All @@ -73,8 +73,8 @@ void L1HLTTauMatching::produce(edm::StreamID iSId, edm::Event& iEvent, const edm
for (unsigned int iJet = 0; iJet < tauJets->size(); iJet++) {
const PFTau& myJet = (*tauJets)[iJet];
//Find the relative L2TauJets, to see if it has been reconstructed
deltaR = ROOT::Math::VectorUtil::DeltaR(myJet.p4().Vect(), (jetCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR < matchingR) {
double deltaR2 = ROOT::Math::VectorUtil::DeltaR2(myJet.p4().Vect(), (jetCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR2 < matchingR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (deltaR2 < matchingR) {
if (deltaR2 < matchingR2) {

// LeafCandidate myLC(myJet);
if (myJet.leadChargedHadrCand().isNonnull()) {
a = myJet.leadChargedHadrCand()->vertex();
Expand Down
7 changes: 3 additions & 4 deletions RecoTauTag/HLTProducers/src/L1THLTTauMatching.cc
Expand Up @@ -26,8 +26,7 @@ L1THLTTauMatching::~L1THLTTauMatching() {}
void L1THLTTauMatching::produce(edm::StreamID iSId, edm::Event& iEvent, const edm::EventSetup& iES) const {
unique_ptr<PFTauCollection> tauL2jets(new PFTauCollection);

double deltaR = 1.0;
double matchingR = 0.5;
constexpr double matchingR = 0.5 * 0.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
constexpr double matchingR = 0.5 * 0.5;
constexpr double matchingR2 = 0.5 * 0.5;


// Getting HLT jets to be matched
edm::Handle<PFTauCollection> tauJets;
Expand All @@ -45,8 +44,8 @@ void L1THLTTauMatching::produce(edm::StreamID iSId, edm::Event& iEvent, const ed
for (unsigned int iJet = 0; iJet < tauJets->size(); iJet++) {
// Find the relative L2TauJets, to see if it has been reconstructed
const PFTau& myJet = (*tauJets)[iJet];
deltaR = ROOT::Math::VectorUtil::DeltaR(myJet.p4().Vect(), (tauCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR < matchingR) {
double deltaR2 = ROOT::Math::VectorUtil::DeltaR2(myJet.p4().Vect(), (tauCandRefVec[iL1Tau]->p4()).Vect());
if (deltaR2 < matchingR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (deltaR2 < matchingR) {
if (deltaR2 < matchingR2) {

if (myJet.leadChargedHadrCand().isNonnull()) {
a = myJet.leadChargedHadrCand()->vertex();
}
Expand Down
4 changes: 2 additions & 2 deletions RecoTauTag/HLTProducers/src/L2TauJetsMerger.cc
Expand Up @@ -58,8 +58,8 @@ void L2TauJetsMerger::produce(edm::StreamID iSId, edm::Event& iEvent, const edm:
tauL2jets->push_back(myTmpJets[0]);
CaloJetCollection tmp;
for (unsigned int i = 1; i < myTmpJets.size(); ++i) {
double DR = ROOT::Math::VectorUtil::DeltaR(myTmpJets[0].p4(), myTmpJets[i].p4());
if (DR > 0.1)
double DR2 = ROOT::Math::VectorUtil::DeltaR2(myTmpJets[0].p4(), myTmpJets[i].p4());
if (DR2 > 0.1 * 0.1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (DR2 > 0.1 * 0.1)
if (DR2 > 0.01)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I prefer the cleaner expression of mine. For the compiler they'll be the same anyhow

tmp.push_back(myTmpJets[i]);
}
myTmpJets.swap(tmp);
Expand Down
15 changes: 8 additions & 7 deletions RecoTauTag/HLTProducers/src/PFJetsMaxInvMassModule.cc
@@ -1,8 +1,10 @@
#include "RecoTauTag/HLTProducers/interface/PFJetsMaxInvMassModule.h"
#include "Math/GenVector/VectorUtil.h"
#include "DataFormats/HLTReco/interface/TriggerTypeDefs.h"
#include "FWCore/Utilities/interface/EDMException.h"
#include "CommonTools/Utils/interface/PtComparator.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "FWCore/ParameterSet/interface/ParameterSetDescription.h"
#include "FWCore/Utilities/interface/InputTag.h"
#include "DataFormats/Common/interface/Handle.h"

//
// class declaration
Expand All @@ -13,7 +15,6 @@ PFJetsMaxInvMassModule::PFJetsMaxInvMassModule(const edm::ParameterSet& iConfig)
removeMaxInvMassPair_(iConfig.getParameter<bool>("removeMaxInvMassPair")) {
produces<reco::PFJetCollection>();
}
PFJetsMaxInvMassModule::~PFJetsMaxInvMassModule() {}

void PFJetsMaxInvMassModule::produce(edm::StreamID iSId, edm::Event& iEvent, const edm::EventSetup& iES) const {
std::unique_ptr<reco::PFJetCollection> addPFJets(new reco::PFJetCollection);
Expand All @@ -23,14 +24,14 @@ void PFJetsMaxInvMassModule::produce(edm::StreamID iSId, edm::Event& iEvent, con

unsigned iCan = 0;
unsigned jCan = 0;
double mjj_max = 0;
double m2jj_max = 0;

if (jets->size() > 1) {
for (unsigned i = 0; i < jets->size(); i++) {
for (unsigned j = i + 1; j < jets->size(); j++) {
double test = ((*jets)[i].p4() + (*jets)[j].p4()).M();
if (test > mjj_max) {
mjj_max = test;
double test = ((*jets)[i].p4() + (*jets)[j].p4()).M2();
if (test > m2jj_max) {
m2jj_max = test;
iCan = i;
jCan = j;
}
Expand Down