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.

Sign up for GitHub

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

Jump to bottom

RecHit-based rho producer for HLT #36157

Merged
merged 10 commits into from
Nov 26, 2021
Merged

RecHit-based rho producer for HLT #36157

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9710cd0
new rho producer for HLT
swagata87 Nov 17, 2021
0a95f54
code cleanup
swagata87 Nov 17, 2021
da552c9
added supporting text
swagata87 Nov 17, 2021
e4ae3d2
addressed review comments
swagata87 Nov 18, 2021
73db4a6
Addressed comments from Vincenzo and Joosep
swagata87 Nov 20, 2021
f10d161
code-check
swagata87 Nov 20, 2021
97d4ff2
Address comments from Marino and Vincenzo
swagata87 Nov 21, 2021
b4d3d4f
code check
swagata87 Nov 21, 2021
503d8b4
remove unnecessary headers
swagata87 Nov 23, 2021
3d014b7
add packages to BuildFile
swagata87 Nov 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RecoJets/JetProducers/BuildFile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<use name="SimDataFormats/Vertex"/>
<use name="SimDataFormats/Track"/>
<use name="RecoJets/JetAlgorithms"/>
<use name="CondFormats/EcalObjects"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this should go to RecoJets/JetProducers/plugins/BuildFile.xml instead.

<use name="JetMETCorrections/JetCorrector"/>
<use name="DataFormats/CastorReco"/>
<use name="CommonTools/MVAUtils"/>
Expand Down
180 changes: 180 additions & 0 deletions RecoJets/JetProducers/plugins/FixedGridRhoProducerFastjetFromRecHit.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
/*

Author: Swagata Mukherjee

Date: November 2021

This producer computes rho from ECAL and HCAL recHits.
The current plan is to use it in egamma and muon HLT, who currently use
the other producer called FixedGridRhoProducerFastjet.
At HLT, FixedGridRhoProducerFastjet takes calotowers as input and computes rho.
But calotowers are expected to be phased out sometime in Run3.
So this recHit-based rho producer, FixedGridRhoProducerFastjetFromRecHit, can be used as an alternative.

*/

#include "FWCore/Framework/interface/stream/EDProducer.h"
#include "FWCore/Framework/interface/Frameworkfwd.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "fastjet/tools/GridMedianBackgroundEstimator.hh"
#include "FWCore/Framework/interface/Event.h"
#include "DataFormats/Common/interface/View.h"
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
#include "DataFormats/Common/interface/View.h"

#include "FWCore/Framework/interface/MakerMacros.h"
#include "DataFormats/HcalRecHit/interface/HcalRecHitCollections.h"
#include "TLorentzVector.h"
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
#include "TLorentzVector.h"

General comment: I was wondering if using TLorentzVector is the best choice here. I would have naively picked math::XYZTLorentzVector (used in reco::Candidate), or even just used 4 floats (px/py/pz/E), as that seems to be all that's needed below in this plugin.

#include "Geometry/CaloGeometry/interface/CaloGeometry.h"
#include "Geometry/CaloGeometry/interface/CaloSubdetectorGeometry.h"
#include "Geometry/Records/interface/CaloGeometryRecord.h"
#include "DataFormats/Math/interface/Vector3D.h"
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
#include "DataFormats/Math/interface/Vector3D.h"

#include "DataFormats/EcalRecHit/interface/EcalRecHitCollections.h"
#include "RecoEgamma/EgammaIsolationAlgos/interface/EgammaHcalIsolation.h"
#include "CondFormats/EcalObjects/interface/EcalPFRecHitThresholds.h"
#include "CondFormats/DataRecord/interface/EcalPFRecHitThresholdsRcd.h"

class FixedGridRhoProducerFastjetFromRecHit : public edm::stream::EDProducer<> {
public:
explicit FixedGridRhoProducerFastjetFromRecHit(const edm::ParameterSet &iConfig);
~FixedGridRhoProducerFastjetFromRecHit() override;
static void fillDescriptions(edm::ConfigurationDescriptions &descriptions);

private:
void produce(edm::Event &, const edm::EventSetup &) override;
void getHitP4(const DetId &detId, float hitE, TLorentzVector &hitp4, const CaloGeometry &caloGeometry);
bool passedHcalNoiseCut(const HBHERecHit &hit);
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
bool passedHcalNoiseCut(const HBHERecHit &hit);
bool passedHcalNoiseCut(const HBHERecHit &hit) const;

bool passedEcalNoiseCut(const EcalRecHit &hit, const EcalPFRecHitThresholds *thresholds);
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
bool passedEcalNoiseCut(const EcalRecHit &hit, const EcalPFRecHitThresholds *thresholds);
bool passedEcalNoiseCut(const EcalRecHit &hit, const EcalPFRecHitThresholds& thresholds);


fastjet::GridMedianBackgroundEstimator bge_;
edm::EDGetTokenT<HBHERecHitCollection> hbheRecHitsTag_;
edm::EDGetTokenT<EcalRecHitCollection> ebRecHitsTag_;
edm::EDGetTokenT<EcalRecHitCollection> eeRecHitsTag_;
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
edm::EDGetTokenT<HBHERecHitCollection> hbheRecHitsTag_;
edm::EDGetTokenT<EcalRecHitCollection> ebRecHitsTag_;
edm::EDGetTokenT<EcalRecHitCollection> eeRecHitsTag_;
const edm::EDGetTokenT<HBHERecHitCollection> hbheRecHitsTag_;
const edm::EDGetTokenT<EcalRecHitCollection> ebRecHitsTag_;
const edm::EDGetTokenT<EcalRecHitCollection> eeRecHitsTag_;


const EgammaHcalIsolation::arrayHB eThresHB_;
const EgammaHcalIsolation::arrayHE eThresHE_;

//Muon HLT currently use ECAL-only rho for ECAL isolation,
//and HCAL-only rho for HCAL isolation. So, this skipping option is needed.
bool skipHCAL_;
bool skipECAL_;
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
bool skipHCAL_;
bool skipECAL_;
const bool skipHCAL_;
const bool skipECAL_;


const edm::ESGetToken<EcalPFRecHitThresholds, EcalPFRecHitThresholdsRcd> ecalPFRechitThresholdsToken_;
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
const edm::ESGetToken<EcalPFRecHitThresholds, EcalPFRecHitThresholdsRcd> ecalPFRechitThresholdsToken_;
const edm::ESGetToken<EcalPFRecHitThresholds, EcalPFRecHitThresholdsRcd> ecalPFRecHitThresholdsToken_;

Consistent naming (RecHit).

const edm::ESGetToken<CaloGeometry, CaloGeometryRecord> caloGeometryToken_;
};

FixedGridRhoProducerFastjetFromRecHit::FixedGridRhoProducerFastjetFromRecHit(const edm::ParameterSet &iConfig)
: bge_(iConfig.getParameter<double>("maxRapidity"), iConfig.getParameter<double>("gridSpacing")),
hbheRecHitsTag_(consumes(iConfig.getParameter<edm::InputTag>("hbheRecHitsTag"))),
ebRecHitsTag_(consumes(iConfig.getParameter<edm::InputTag>("ebRecHitsTag"))),
eeRecHitsTag_(consumes(iConfig.getParameter<edm::InputTag>("eeRecHitsTag"))),
eThresHB_(iConfig.getParameter<EgammaHcalIsolation::arrayHB>("eThresHB")),
eThresHE_(iConfig.getParameter<EgammaHcalIsolation::arrayHE>("eThresHE")),
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering what happens if the input vector from the PSet does not have the size expected by arrayHB (4) and arrayHE (7). If this fails silently, it might be worth adding an explicitly check on the size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a good point, I am now checking what happens if I provide 2 elements instead for 4 for HB. It raise the following error and does not run; so I'm not adding any explicit check on size.

----- Begin Fatal Exception 18-Nov-2021 11:12:35 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=FixedGridRhoProducerFastjetFromRecHit label='hltRhoNew'
Exception Message:
The parameter 'eThresHB' should have 4 elements, but has 2 elements in the configuration.
----- End Fatal Exception -------------------------------------------------

skipHCAL_(iConfig.getParameter<bool>("skipHCAL")),
skipECAL_(iConfig.getParameter<bool>("skipECAL")),
ecalPFRechitThresholdsToken_{esConsumes()},
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
ecalPFRechitThresholdsToken_{esConsumes()},
ecalPFRecHitThresholdsToken_{esConsumes()},

Consistent naming (RecHit).

caloGeometryToken_{esConsumes()} {
produces<double>();

This comment was marked as resolved.

Sign in to view

}

void FixedGridRhoProducerFastjetFromRecHit::fillDescriptions(edm::ConfigurationDescriptions &descriptions) {
edm::ParameterSetDescription desc;
//We use raw recHits and not the PF recHits, because in Phase1 muon and egamma HLT,
//PF recHit collection is regional, while the full raw recHit collection is available.
desc.add<edm::InputTag>("hbheRecHitsTag", edm::InputTag("hltHbhereco"));
desc.add<edm::InputTag>("ebRecHitsTag", edm::InputTag("hltEcalRecHit", "EcalRecHitsEB"));
desc.add<edm::InputTag>("eeRecHitsTag", edm::InputTag("hltEcalRecHit", "EcalRecHitsEE"));
desc.add<bool>("skipHCAL", false);
desc.add<bool>("skipECAL", false);
//eThresHB/HE are from RecoParticleFlow/PFClusterProducer/python/particleFlowRecHitHBHE_cfi.py
desc.add<std::vector<double> >("eThresHB", {0.1, 0.2, 0.3, 0.3});
desc.add<std::vector<double> >("eThresHE", {0.1, 0.2, 0.2, 0.2, 0.2, 0.2, 0.2});
desc.add<double>("maxRapidity", 2.5);
desc.add<double>("gridSpacing", 0.55);
descriptions.add("hltFixedGridRhoProducerFastjetFromRecHit", desc);
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
descriptions.add("hltFixedGridRhoProducerFastjetFromRecHit", desc);
descriptions.addWithDefaultLabel(desc);

}

FixedGridRhoProducerFastjetFromRecHit::~FixedGridRhoProducerFastjetFromRecHit() {}
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
FixedGridRhoProducerFastjetFromRecHit::~FixedGridRhoProducerFastjetFromRecHit() {}
FixedGridRhoProducerFastjetFromRecHit::~FixedGridRhoProducerFastjetFromRecHit() = default;


void FixedGridRhoProducerFastjetFromRecHit::produce(edm::Event &iEvent, const edm::EventSetup &iSetup) {
std::vector<fastjet::PseudoJet> inputs;
auto const &thresholds = iSetup.getData(ecalPFRechitThresholdsToken_);
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
auto const &thresholds = iSetup.getData(ecalPFRechitThresholdsToken_);
auto const &thresholds = iSetup.getData(ecalPFRecHitThresholdsToken_);

Consistent naming (RecHit).


if (skipHCAL_ && skipECAL_) {
throw cms::Exception("FixedGridRhoProducerFastjetFromRecHit")
<< "skipHCAL and skipECAL both can't be True. Make at least one of them False.";
}
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 (skipHCAL_ && skipECAL_) {
throw cms::Exception("FixedGridRhoProducerFastjetFromRecHit")
<< "skipHCAL and skipECAL both can't be True. Make at least one of them False.";
}

This can be moved to the ctor.


This comment was marked as resolved.

Sign in to view

if (!skipHCAL_) {
for (const auto &hit : iEvent.get(hbheRecHitsTag_)) {
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
for (const auto &hit : iEvent.get(hbheRecHitsTag_)) {
auto const& hbheRecHits = iEvent.get(hbheRecHitsTag_);
inputs.reserve(inputs.size() + hbheRecHits.size());
for (const auto &hit : hbheRecHits) {

if (passedHcalNoiseCut(hit)) {
TLorentzVector hitp4(0, 0, 0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above on use of TLorentzVector.

getHitP4(hit.id(), hit.energy(), hitp4, iSetup.getData(caloGeometryToken_));
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
getHitP4(hit.id(), hit.energy(), hitp4, iSetup.getData(caloGeometryToken_));
getHitP4(hit.id(), hit.energy(), hitp4, caloGeometry);

inputs.push_back(fastjet::PseudoJet(hitp4.Px(), hitp4.Py(), hitp4.Pz(), hitp4.E()));
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
inputs.emplace_back(hitp4.Px(), hitp4.Py(), hitp4.Pz(), hitp4.E());
similar below

Copy link
Contributor

Choose a reason for hiding this comment

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

there is no need of a LorentzVector or similar.
one single double hitp4[4]; suffice;

}
}
}

if (!skipECAL_) {
for (const auto &hit : iEvent.get(ebRecHitsTag_)) {
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
for (const auto &hit : iEvent.get(ebRecHitsTag_)) {
auto const& ebRecHits = iEvent.get(ebRecHitsTag_);
inputs.reserve(inputs.size() + ebRecHits.size());
for (const auto &hit : ebRecHits) {

if (passedEcalNoiseCut(hit, &thresholds)) {
TLorentzVector hitp4(0, 0, 0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above on use of TLorentzVector.

getHitP4(hit.id(), hit.energy(), hitp4, iSetup.getData(caloGeometryToken_));
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
getHitP4(hit.id(), hit.energy(), hitp4, iSetup.getData(caloGeometryToken_));
getHitP4(hit.id(), hit.energy(), hitp4, caloGeometry);

inputs.push_back(fastjet::PseudoJet(hitp4.Px(), hitp4.Py(), hitp4.Pz(), hitp4.E()));
}
}

for (const auto &hit : iEvent.get(eeRecHitsTag_)) {
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
for (const auto &hit : iEvent.get(eeRecHitsTag_)) {
auto const& eeRecHits = iEvent.get(eeRecHitsTag_);
inputs.reserve(inputs.size() + eeRecHits.size());
for (const auto &hit : eeRecHits) {

if (passedEcalNoiseCut(hit, &thresholds)) {
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 (passedEcalNoiseCut(hit, &thresholds)) {
if (passedEcalNoiseCut(hit, thresholds)) {

TLorentzVector hitp4(0, 0, 0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above on use of TLorentzVector.

getHitP4(hit.id(), hit.energy(), hitp4, iSetup.getData(caloGeometryToken_));
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
getHitP4(hit.id(), hit.energy(), hitp4, iSetup.getData(caloGeometryToken_));
getHitP4(hit.id(), hit.energy(), hitp4, caloGeometry);

inputs.push_back(fastjet::PseudoJet(hitp4.Px(), hitp4.Py(), hitp4.Pz(), hitp4.E()));
}
}
}

bge_.set_particles(inputs);
iEvent.put(std::make_unique<double>(bge_.rho()));
}

void FixedGridRhoProducerFastjetFromRecHit::getHitP4(const DetId &detId,
float hitE,
TLorentzVector &hitp4,
const CaloGeometry &caloGeometry) {
const CaloSubdetectorGeometry *subDetGeom = caloGeometry.getSubdetectorGeometry(detId);
std::shared_ptr<const CaloCellGeometry> cellGeom =
subDetGeom != nullptr ? subDetGeom->getGeometry(detId) : std::shared_ptr<const CaloCellGeometry>();
if (cellGeom != nullptr) {
const auto &gpPos = cellGeom->repPos();
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
std::shared_ptr<const CaloCellGeometry> cellGeom =
subDetGeom != nullptr ? subDetGeom->getGeometry(detId) : std::shared_ptr<const CaloCellGeometry>();
if (cellGeom != nullptr) {
const auto &gpPos = cellGeom->repPos();
if (subDetGeom != nullptr) {
const auto &gpPos = subDetGeom->getGeometry(detId)->repPos();

This is a guess, just to highlight that maybe things could be simplified (..but maybe I'm missing something). Mainly, I didn't get why shared_ptr was needed.

double thispt = hitE / cosh(gpPos.eta());
Copy link
Contributor

Choose a reason for hiding this comment

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

these could all be const

double thispx = thispt * cos(gpPos.phi());
double thispy = thispt * sin(gpPos.phi());
double thispz = thispt * sinh(gpPos.eta());
hitp4.SetPxPyPzE(thispx, thispy, thispz, hitE);
Copy link
Contributor

Choose a reason for hiding this comment

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

should it not be corrected for beam spot (or identified signal vertex) position?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this rho producer, we are just aiming for a proxy of pile-up. As long as we can construct a variable whose value increases with pile-up, we are fine. So I think it's okay to not correct the recHit positions for beamspot.

I have addressed the other comments.

} else {
if (detId.rawId() != 0)
Copy link
Contributor

@VinInn VinInn Nov 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

again: is this really something that can happen?
in particular once the hit paid the noise cut!

edm::LogInfo("FixedGridRhoProducerFastjetFromRecHit")
<< "Warning : Geometry not found for a calo hit, setting p4 as (0,0,0,0)" << std::endl;
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
edm::LogInfo("FixedGridRhoProducerFastjetFromRecHit")
<< "Warning : Geometry not found for a calo hit, setting p4 as (0,0,0,0)" << std::endl;
edm::LogWarning("FixedGridRhoProducerFastjetFromRecHit")
<< "Geometry not found for a calo hit, setting p4 as (0,0,0,0)" << std::endl;

Unclear to me if this should be a warning, or an "info".

hitp4.SetPxPyPzE(0, 0, 0, 0);
}
}

//HCAL noise cleaning cuts.
bool FixedGridRhoProducerFastjetFromRecHit::passedHcalNoiseCut(const HBHERecHit &hit) {
bool passed = false;
const HcalDetId thisDetId(hit.detid());
const int thisDepth = thisDetId.depth();
if ((thisDetId.subdet() == HcalBarrel) && (hit.energy() > eThresHB_[thisDepth - 1]))
passed = true;
else if ((thisDetId.subdet() == HcalEndcap) && (hit.energy() > eThresHE_[thisDepth - 1]))
passed = true;
return passed;
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
bool passed = false;
const HcalDetId thisDetId(hit.detid());
const int thisDepth = thisDetId.depth();
if ((thisDetId.subdet() == HcalBarrel) && (hit.energy() > eThresHB_[thisDepth - 1]))
passed = true;
else if ((thisDetId.subdet() == HcalEndcap) && (hit.energy() > eThresHE_[thisDepth - 1]))
passed = true;
return passed;
const auto thisDetId = hit.detid();
const auto thisDepth = thisDetId.depth();
if (thisDetId.subdet() == HcalBarrel && hit.energy() > eThresHB_[thisDepth-1])
return true;
else if (thisDetId.subdet() == HcalEndcap && hit.energy() > eThresHE_[thisDepth-1])
return true;
return false;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have now changed it as you suggested; just one minor thing is that,
I have done
const HcalDetId thisDetId = hit.detid();
instead of
const auto thisDetId = hit.detid();
Unless I explicitly specify that thisDetId is of type HcalDetId, the compiler seem to think that thisDetId is of type DetId.
HcalDetId inherits from DetId, but depth() which is accessed in next line, is available only in HcalDetId and not in DetId. So it raises an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, no problem. Just FYI, it looks like you could use HBHERecHit::id() (in any case, this is RECO, so not my domain).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, const auto thisDetId = hit.id(); works fine; so moving to that..

and all other comments are also addressed now. Will push the updated version soon.

}

//ECAL noise cleaning cuts using per-crystal PF-recHit thresholds.
bool FixedGridRhoProducerFastjetFromRecHit::passedEcalNoiseCut(const EcalRecHit &hit,
const EcalPFRecHitThresholds *thresholds) {
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
const EcalPFRecHitThresholds *thresholds) {
const EcalPFRecHitThresholds& thresholds) {

bool passed = false;
if (hit.energy() > (*thresholds)[hit.detid()])
passed = true;
return passed;
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
bool passed = false;
if (hit.energy() > (*thresholds)[hit.detid()])
passed = true;
return passed;
return (hit.energy() > thresholds[hit.detid()]);

}

DEFINE_FWK_MODULE(FixedGridRhoProducerFastjetFromRecHit);