Skip to content

Fix HLTEcalPhiSymFilter and add spike cleaning option #49852

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

Merged
cmsbuild merged 1 commit into from
Jan 21, 2026
Merged

Fix HLTEcalPhiSymFilter and add spike cleaning option #49852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions HLTrigger/Configuration/python/customizeHLTforCMSSW.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,14 @@ def customizeHLTfor49799(process):

return process

def customizeHLTfor49852(process):
for prod in producers_by_type(process, 'HLTEcalPhiSymFilter'):
if not hasattr(prod, 'cleanReco'):
setattr(prod, 'cleanReco', cms.bool(False))
if not hasattr(prod, 'cleaningConfig'):
from RecoLocalCalo.EcalRecAlgos.ecalCleaningAlgo import cleaningAlgoConfig
setattr(prod, 'cleaningConfig', cleaningAlgoConfig)
Copy link
Contributor

@Martin-Grunewald Martin-Grunewald Jan 23, 2026

Choose a reason for hiding this comment

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

The cleaningConfig imported here is arsing from a ~12 year old file: https://github.com/cms-sw/cmssw/blob/master/RecoLocalCalo/EcalRecAlgos/python/ecalCleaningAlgo.py while what you code in the fillDescriptions method down below seems way more up-to-date (and as used elsewhere). So I assume we should use the latter.
Also, since fillDescriptions provides both cleanReco as well as cleaningConfig, there should not be the need for any of the customisation above, correct? Of course, the PRs are already merged, but just to clarify on the settings!

Copy link
Contributor Author

@thomreis thomreis Jan 23, 2026

Choose a reason for hiding this comment

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

If the missing parameters in existing menus are filled in from the default values in the fillDescriptions then this customization not needed indeed.
The cleaning parameters from fillDescriptions are the up to date ones and the ones from cleaningAlgoConfig were just meant as placeholders under the assumption that cleanReco would be False whenever cleaningConfig was not present.

Copy link
Contributor

@Martin-Grunewald Martin-Grunewald Jan 23, 2026

Choose a reason for hiding this comment

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

Thanks for the clarification!

Copy link
Contributor

@mmusich mmusich Jan 23, 2026

Choose a reason for hiding this comment

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

indeed

diff --git a/HLTrigger/Configuration/python/customizeHLTforCMSSW.py b/HLTrigger/Configuration/python/customizeHLTforCMSSW.py
index ac5d2bcd684..b110405746b 100644
--- a/HLTrigger/Configuration/python/customizeHLTforCMSSW.py
+++ b/HLTrigger/Configuration/python/customizeHLTforCMSSW.py
@@ -228,6 +228,6 @@ def customizeHLTforCMSSW(process, menuType="GRun"):
     # process = customizeHLTfor49436(process)
 
     process = customizeHLTfor49799(process)
-    process = customizeHLTfor49852(process)
+    #process = customizeHLTfor49852(process)
 
     return process

passes addOnTests.py just fine.

Of course, the PRs are already merged, but just to clarify on the settings!

shall we remove it now?

Copy link
Contributor Author

@thomreis thomreis Jan 23, 2026

Choose a reason for hiding this comment

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

Can be removed yes.

Copy link
Contributor Author

@thomreis thomreis Jan 23, 2026

Choose a reason for hiding this comment

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

#49923

Copy link
Contributor Author

@thomreis thomreis Jan 23, 2026

Choose a reason for hiding this comment

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

And the backport #49924

return process

# CMSSW version specific customizations
def customizeHLTforCMSSW(process, menuType="GRun"):
Expand All @@ -220,5 +228,6 @@ def customizeHLTforCMSSW(process, menuType="GRun"):
# process = customizeHLTfor49436(process)

process = customizeHLTfor49799(process)
process = customizeHLTfor49852(process)

return process
1 change: 1 addition & 0 deletions HLTrigger/special/plugins/BuildFile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
<use name="Geometry/Records"/>
<use name="Geometry/TrackerGeometryBuilder"/>
<use name="HLTrigger/HLTcore"/>
<use name="RecoLocalCalo/EcalRecAlgos"/>
<use name="RecoEcal/EgammaClusterAlgos"/>
<use name="RecoEcal/EgammaCoreTools"/>
<use name="TrackingTools/Records"/>
Expand Down
118 changes: 88 additions & 30 deletions HLTrigger/special/plugins/HLTEcalPhiSymFilter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,38 @@
#include "Geometry/CaloGeometry/interface/CaloSubdetectorGeometry.h"

HLTEcalPhiSymFilter::HLTEcalPhiSymFilter(const edm::ParameterSet& config)
: ecalChannelStatusRcdToken_(esConsumes()),
: statusThreshold_(config.getParameter<uint32_t>("statusThreshold")),
useRecoFlag_(config.getParameter<bool>("useRecoFlag")),
cleanReco_(config.getParameter<bool>("cleanReco")),
ampCut_barlP_(config.getParameter<std::vector<double> >("ampCut_barrelP")),
ampCut_barlM_(config.getParameter<std::vector<double> >("ampCut_barrelM")),
ampCut_endcP_(config.getParameter<std::vector<double> >("ampCut_endcapP")),
ampCut_endcM_(config.getParameter<std::vector<double> >("ampCut_endcapM")),
ecalChannelStatusRcdToken_(useRecoFlag_ ? decltype(ecalChannelStatusRcdToken_)()
: decltype(ecalChannelStatusRcdToken_)(esConsumes())),
caloGeometryRecordToken_(esConsumes()),
barrelDigisToken_(consumes<EBDigiCollection>(config.getParameter<edm::InputTag>("barrelDigiCollection"))),
endcapDigisToken_(consumes<EEDigiCollection>(config.getParameter<edm::InputTag>("endcapDigiCollection"))),
barrelUncalibHitsToken_(
consumes<EcalUncalibratedRecHitCollection>(config.getParameter<edm::InputTag>("barrelUncalibHitCollection"))),
endcapUncalibHitsToken_(
consumes<EcalUncalibratedRecHitCollection>(config.getParameter<edm::InputTag>("endcapUncalibHitCollection"))),
barrelHitsToken_(consumes<EBRecHitCollection>(config.getParameter<edm::InputTag>("barrelHitCollection"))),
endcapHitsToken_(consumes<EERecHitCollection>(config.getParameter<edm::InputTag>("endcapHitCollection"))),
barrelHitsToken_(useRecoFlag_
? consumes<EBRecHitCollection>(config.getParameter<edm::InputTag>("barrelHitCollection"))
: edm::EDGetTokenT<EBRecHitCollection>()),
endcapHitsToken_(useRecoFlag_
? consumes<EERecHitCollection>(config.getParameter<edm::InputTag>("endcapHitCollection"))
: edm::EDGetTokenT<EERecHitCollection>()),
phiSymBarrelDigis_(config.getParameter<std::string>("phiSymBarrelDigiCollection")),
phiSymEndcapDigis_(config.getParameter<std::string>("phiSymEndcapDigiCollection")),
ampCut_barlP_(config.getParameter<std::vector<double> >("ampCut_barrelP")),
ampCut_barlM_(config.getParameter<std::vector<double> >("ampCut_barrelM")),
ampCut_endcP_(config.getParameter<std::vector<double> >("ampCut_endcapP")),
ampCut_endcM_(config.getParameter<std::vector<double> >("ampCut_endcapM")),
statusThreshold_(config.getParameter<uint32_t>("statusThreshold")),
useRecoFlag_(config.getParameter<bool>("useRecoFlag")) {
phiSymEndcapDigis_(config.getParameter<std::string>("phiSymEndcapDigiCollection")) {
//register your products
produces<EBDigiCollection>(phiSymBarrelDigis_);
produces<EEDigiCollection>(phiSymEndcapDigis_);

if (useRecoFlag_ && cleanReco_) {
const auto cleaningPs = config.getParameter<edm::ParameterSet>("cleaningConfig");
cleaningAlgo_ = std::make_unique<EcalCleaningAlgo>(cleaningPs);
}
}

HLTEcalPhiSymFilter::~HLTEcalPhiSymFilter() = default;
Expand Down Expand Up @@ -63,6 +74,26 @@ void HLTEcalPhiSymFilter::fillDescriptions(edm::ConfigurationDescriptions& descr
12., 12., 12., 12., 12., 12., 12., 12., 12., 12., 12., 12., 12.});
desc.add<std::string>("phiSymBarrelDigiCollection", "phiSymEcalDigisEB");
desc.add<std::string>("phiSymEndcapDigiCollection", "phiSymEcalDigisEE");
desc.add<bool>("cleanReco", false);
{
edm::ParameterSetDescription psd0;
psd0.add<double>("e6e2thresh", 0.04);
psd0.add<double>("tightenCrack_e6e2_double", 3);
psd0.add<double>("e4e1Threshold_endcap", 0.3);
psd0.add<double>("tightenCrack_e4e1_single", 3);
psd0.add<double>("tightenCrack_e1_double", 2);
psd0.add<double>("cThreshold_barrel", 4);
psd0.add<double>("e4e1Threshold_barrel", 0.08);
psd0.add<double>("tightenCrack_e1_single", 2);
psd0.add<double>("e4e1_b_barrel", -0.024);
psd0.add<double>("e4e1_a_barrel", 0.04);
psd0.add<double>("ignoreOutOfTimeThresh", 1000000000.0);
psd0.add<double>("cThreshold_endcap", 15);
psd0.add<double>("e4e1_b_endcap", -0.0125);
psd0.add<double>("e4e1_a_endcap", 0.02);
psd0.add<double>("cThreshold_double", 10);
desc.add<edm::ParameterSetDescription>("cleaningConfig", psd0);
}
descriptions.add("hltEcalPhiSymFilter", desc);
}

Expand All @@ -89,43 +120,55 @@ bool HLTEcalPhiSymFilter::filter(edm::StreamID, edm::Event& event, const edm::Ev
Handle<EEDigiCollection> endcapDigisHandle;
Handle<EcalUncalibratedRecHitCollection> barrelUncalibRecHitsHandle;
Handle<EcalUncalibratedRecHitCollection> endcapUncalibRecHitsHandle;
Handle<EBRecHitCollection> barrelRecHitsHandle;
Handle<EERecHitCollection> endcapRecHitsHandle;

event.getByToken(barrelDigisToken_, barrelDigisHandle);
event.getByToken(endcapDigisToken_, endcapDigisHandle);
event.getByToken(barrelUncalibHitsToken_, barrelUncalibRecHitsHandle);
event.getByToken(endcapUncalibHitsToken_, endcapUncalibRecHitsHandle);
event.getByToken(barrelHitsToken_, barrelRecHitsHandle);
event.getByToken(endcapHitsToken_, endcapRecHitsHandle);

//Create empty output collections
std::unique_ptr<EBDigiCollection> phiSymEBDigiCollection(new EBDigiCollection);
std::unique_ptr<EEDigiCollection> phiSymEEDigiCollection(new EEDigiCollection);

const EBDigiCollection* EBDigis = barrelDigisHandle.product();
const EEDigiCollection* EEDigis = endcapDigisHandle.product();
const EBRecHitCollection* EBRechits = barrelRecHitsHandle.product();
const EERecHitCollection* EERechits = endcapRecHitsHandle.product();
// RecHits are only needed when recoFlags are checked
const auto* EBRechits = useRecoFlag_ ? &event.get(barrelHitsToken_) : nullptr;
const auto* EERechits = useRecoFlag_ ? &event.get(endcapHitsToken_) : nullptr;

//Select interesting EcalDigis (barrel)
EcalUncalibratedRecHitCollection::const_iterator itunb;
for (itunb = barrelUncalibRecHitsHandle->begin(); itunb != barrelUncalibRecHitsHandle->end(); itunb++) {
EcalUncalibratedRecHit hit = (*itunb);
EBDetId hitDetId = hit.id();
uint16_t statusCode = 0;
if (useRecoFlag_)
statusCode = EBRechits->find(hitDetId)->recoFlag();
else
statusCode = channelStatus[itunb->id().rawId()].getStatusCode();
int iRing = CalibRing.getRingIndex(hitDetId);
float ampCut = 0.;
if (hitDetId.ieta() < 0)
ampCut = ampCut_barlM_[iRing];
else if (hitDetId.ieta() > 0)
ampCut = ampCut_barlP_[iRing - N_RING_BARREL / 2];
float amplitude = hit.amplitude();
if (statusCode <= statusThreshold_ && amplitude > ampCut) {
if (hit.amplitude() <= ampCut) {
continue;
}

uint32_t statusCode = 0;
if (useRecoFlag_) {
const auto rechit = EBRechits->find(hitDetId);
if (rechit == EBRechits->end()) {
continue;
}
statusCode = rechit->recoFlag();
if (cleanReco_ && cleaningAlgo_) {
const auto flags = cleaningAlgo_->checkTopology(hitDetId, *EBRechits);
if (flags > statusCode) {
statusCode = flags;
}
}
} else {
statusCode = channelStatus[hitDetId.rawId()].getStatusCode();
}

if (statusCode <= statusThreshold_) {
const auto digiIt = EBDigis->find(hitDetId);
if (digiIt != EBDigis->end()) {
phiSymEBDigiCollection->push_back(digiIt->id(), digiIt->begin());
Expand All @@ -141,19 +184,34 @@ bool HLTEcalPhiSymFilter::filter(edm::StreamID, edm::Event& event, const edm::Ev
for (itune = endcapUncalibRecHitsHandle->begin(); itune != endcapUncalibRecHitsHandle->end(); itune++) {
EcalUncalibratedRecHit hit = (*itune);
EEDetId hitDetId = hit.id();
uint16_t statusCode = 0;
if (useRecoFlag_)
statusCode = EERechits->find(hitDetId)->recoFlag();
else
statusCode = channelStatus[itune->id().rawId()].getStatusCode();
int iRing = CalibRing.getRingIndex(hitDetId);
float ampCut = 0.;
if (hitDetId.zside() < 0)
ampCut = ampCut_endcM_[iRing - N_RING_BARREL];
else if (hitDetId.zside() > 0)
ampCut = ampCut_endcP_[iRing - N_RING_BARREL - N_RING_ENDCAP / 2];
float amplitude = hit.amplitude();
if (statusCode <= statusThreshold_ && amplitude > ampCut) {
if (hit.amplitude() <= ampCut) {
continue;
}

uint32_t statusCode = 0;
if (useRecoFlag_) {
const auto rechit = EERechits->find(hitDetId);
if (rechit == EERechits->end()) {
continue;
}
statusCode = rechit->recoFlag();
if (cleanReco_ && cleaningAlgo_) {
const auto flags = cleaningAlgo_->checkTopology(hitDetId, *EERechits);
if (flags > statusCode) {
statusCode = flags;
}
}
} else {
statusCode = channelStatus[hitDetId.rawId()].getStatusCode();
}

if (statusCode <= statusThreshold_) {
const auto digiIt = EEDigis->find(hitDetId);
if (digiIt != EEDigis->end()) {
phiSymEEDigiCollection->push_back(digiIt->id(), digiIt->begin());
Expand Down
24 changes: 16 additions & 8 deletions HLTrigger/special/plugins/HLTEcalPhiSymFilter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
* threshold (e.g. eCut_barl_high_) is applied. If parameter useRecoFlag_
* is true, statusThreshold_ acts on EcalRecHit::recoFlag(), while if it is
* false, it acts on the ChannelStatus record from the database.
* To allow to use tighter cleaning thresholds than the ones used in the
* RecHitProducer the cleanReco_ parameter can be used the specify a
* new cleaningConfig.

*/

Expand All @@ -42,6 +45,7 @@
#include "DataFormats/EcalDigi/interface/EcalDigiCollections.h"
#include "Geometry/CaloGeometry/interface/CaloGeometry.h"
#include "Geometry/Records/interface/CaloGeometryRecord.h"
#include "RecoLocalCalo/EcalRecAlgos/interface/EcalCleaningAlgo.h"

namespace edm {
class ConfigurationDescriptions;
Expand All @@ -60,8 +64,18 @@ class HLTEcalPhiSymFilter : public edm::global::EDFilter<> {
static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);

private:
edm::ESGetToken<EcalChannelStatus, EcalChannelStatusRcd> const ecalChannelStatusRcdToken_;
edm::ESGetToken<CaloGeometry, CaloGeometryRecord> const caloGeometryRecordToken_;
std::unique_ptr<EcalCleaningAlgo> cleaningAlgo_;

const uint32_t statusThreshold_; ///< accept channels with up to this status
const bool useRecoFlag_; ///< use recoflag instead of DB for bad channels
const bool cleanReco_; ///< run cleaning algo and use flag if higher than the reco flag
const std::vector<double> ampCut_barlP_;
const std::vector<double> ampCut_barlM_;
const std::vector<double> ampCut_endcP_;
const std::vector<double> ampCut_endcM_;

const edm::ESGetToken<EcalChannelStatus, EcalChannelStatusRcd> ecalChannelStatusRcdToken_;
const edm::ESGetToken<CaloGeometry, CaloGeometryRecord> caloGeometryRecordToken_;

const edm::EDGetTokenT<EBDigiCollection> barrelDigisToken_;
const edm::EDGetTokenT<EEDigiCollection> endcapDigisToken_;
Expand All @@ -71,12 +85,6 @@ class HLTEcalPhiSymFilter : public edm::global::EDFilter<> {
const edm::EDGetTokenT<EERecHitCollection> endcapHitsToken_;
const std::string phiSymBarrelDigis_;
const std::string phiSymEndcapDigis_;
const std::vector<double> ampCut_barlP_;
const std::vector<double> ampCut_barlM_;
const std::vector<double> ampCut_endcP_;
const std::vector<double> ampCut_endcM_;
const uint32_t statusThreshold_; ///< accept channels with up to this status
const bool useRecoFlag_; ///< use recoflag instead of DB for bad channels
};

#endif