From 6dd4feed1f9ad156d07c07bc91552ea75adf31a9 Mon Sep 17 00:00:00 2001 From: Apurva Narde <58493193+Steepspace@users.noreply.github.com> Date: Tue, 12 May 2026 22:19:12 -0400 Subject: [PATCH 1/5] CaloTowerStatus: Refactor hot tower identification and add default threshold check - Added a logical split to handle the default behavior: if the z-score threshold is untouched, the code now relies strictly on the non-zero hotMap values. - Refactored the complex if-statement for custom thresholds into an if/else block using descriptive boolean variables (`is_dead`, `exceeds_zscore_limit`, `is_low_yield_cold`) for better readability. - Replaced the C-style `std::fabs` with the modern C++ standard `std::abs`. - Guarded the entire evaluation block behind `m_doHotMap` to improve clarity. --- offline/packages/CaloReco/CaloTowerStatus.cc | 28 ++++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/offline/packages/CaloReco/CaloTowerStatus.cc b/offline/packages/CaloReco/CaloTowerStatus.cc index f9e63348b6..b7827ea32a 100644 --- a/offline/packages/CaloReco/CaloTowerStatus.cc +++ b/offline/packages/CaloReco/CaloTowerStatus.cc @@ -234,12 +234,30 @@ int CaloTowerStatus::process_event(PHCompositeNode * /*topNode*/) { m_raw_towers->get_tower_at_channel(channel)->set_isHot(true); } - if (( hotMap_val == 1 || // dead - std::fabs(z_score) > z_score_threshold || // hot or cold - (hotMap_val == 3 && z_score >= -1 * z_score_threshold_default)) // cold part 2 - && m_doHotMap) + if (m_doHotMap) { - m_raw_towers->get_tower_at_channel(channel)->set_isHot(true); + bool is_hot_tower = false; + + // 1. Default behavior: simply rely on the hotMap value + if (z_score_threshold == z_score_threshold_default) + { + is_hot_tower = (hotMap_val != 0); + } + // 2. Custom behavior: evaluate based on the custom z_score threshold + else + { + bool is_dead = (hotMap_val == 1); + bool exceeds_zscore_limit = (std::abs(z_score) > z_score_threshold); // Captures both hot and cold by sigma + bool is_low_yield_cold = (hotMap_val == 3 && z_score >= -1 * z_score_threshold_default); // Captures the mean-based cold towers + + is_hot_tower = (is_dead || exceeds_zscore_limit || is_low_yield_cold); + } + + // Apply the result + if (is_hot_tower) + { + m_raw_towers->get_tower_at_channel(channel)->set_isHot(true); + } } if (chi2 > std::min(std::max(badChi2_treshold_const, adc * adc * badChi2_treshold_quadratic),badChi2_treshold_max)) { From 6589011221aabb717be317596a87dae67e0156bb Mon Sep 17 00:00:00 2001 From: Apurva Narde <58493193+Steepspace@users.noreply.github.com> Date: Wed, 13 May 2026 14:26:29 -0400 Subject: [PATCH 2/5] Bad Tower Status: Use Positive Status Using hotMap_val != 0 treats the CDB missing-value as a hot tower: CDBTTree::GetIntValue returns std::numeric_limits::min() when the channel or field is absent, and that value is nonzero. This is a behavior regression from the previous explicit status checks and could mask towers if a calibration entry is missing or malformed; limit this default path to valid positive/known status codes instead of any nonzero value. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- offline/packages/CaloReco/CaloTowerStatus.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/offline/packages/CaloReco/CaloTowerStatus.cc b/offline/packages/CaloReco/CaloTowerStatus.cc index b7827ea32a..998244ecf0 100644 --- a/offline/packages/CaloReco/CaloTowerStatus.cc +++ b/offline/packages/CaloReco/CaloTowerStatus.cc @@ -238,10 +238,10 @@ int CaloTowerStatus::process_event(PHCompositeNode * /*topNode*/) { bool is_hot_tower = false; - // 1. Default behavior: simply rely on the hotMap value + // 1. Default behavior: rely on valid positive hotMap status codes only if (z_score_threshold == z_score_threshold_default) { - is_hot_tower = (hotMap_val != 0); + is_hot_tower = (hotMap_val > 0); } // 2. Custom behavior: evaluate based on the custom z_score threshold else From ea5838429ff86f205058cdf062dd1d9f728fc499 Mon Sep 17 00:00:00 2001 From: Apurva Narde <58493193+Steepspace@users.noreply.github.com> Date: Wed, 13 May 2026 14:53:50 -0400 Subject: [PATCH 3/5] CaloTowerStatus: Refactor CDBTTree management for memory efficiency Refactored the calibration loading process to ensure heavy tree objects are freed from memory immediately after use. - Moved m_cdbttree_chi2 and m_cdbttree_hotMap from class members to local variables within InitRun to minimize the memory footprint during the event processing loop. - Updated the LoadCalib method to accept CDBTTree pointers as arguments. - Implemented immediate deletion of the tree objects after populating the m_cdbInfo_vec cache. - Simplified the class destructor as it no longer needs to manage these pointers. --- offline/packages/CaloReco/CaloTowerStatus.cc | 35 +++++++++++--------- offline/packages/CaloReco/CaloTowerStatus.h | 5 +-- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/offline/packages/CaloReco/CaloTowerStatus.cc b/offline/packages/CaloReco/CaloTowerStatus.cc index 998244ecf0..0727ba735d 100644 --- a/offline/packages/CaloReco/CaloTowerStatus.cc +++ b/offline/packages/CaloReco/CaloTowerStatus.cc @@ -47,8 +47,6 @@ CaloTowerStatus::~CaloTowerStatus() { std::cout << "CaloTowerStatus::~CaloTowerStatus() Calling dtor" << std::endl; } - delete m_cdbttree_chi2; - delete m_cdbttree_hotMap; } //____________________________________________________________________________.. @@ -78,6 +76,8 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) m_detector = "SEPD"; } + CDBTTree *cdbttree_chi2 = nullptr; + m_calibName_chi2 = m_detector + "_hotTowers_fracBadChi2"; m_fieldname_chi2 = "fraction"; @@ -86,20 +86,20 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) { calibdir_chi2 = m_directURL_chi2; std::cout << "CaloTowerStatus::InitRun: Using direct URL override for chi2: " << calibdir_chi2 << std::endl; - m_cdbttree_chi2 = new CDBTTree(calibdir_chi2); + cdbttree_chi2 = new CDBTTree(calibdir_chi2); } else { calibdir_chi2 = CDBInterface::instance()->getUrl(m_calibName_chi2); if (!calibdir_chi2.empty()) { - m_cdbttree_chi2 = new CDBTTree(calibdir_chi2); + cdbttree_chi2 = new CDBTTree(calibdir_chi2); if (Verbosity() > 0) { std::cout << "CaloTowerStatus::InitRun Found " << m_calibName_chi2 << " Doing isHot for frac bad chi2" << std::endl; } } - else + else { if (m_doAbortNoChi2) { @@ -114,6 +114,8 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) } } + CDBTTree *cdbttree_hotMap = nullptr; + m_calibName_hotMap = m_detector + "_BadTowerMap"; m_fieldname_hotMap = "status"; m_fieldname_z_score = m_detector + "_sigma"; @@ -123,14 +125,14 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) { calibdir_hotMap = m_directURL_hotMap; std::cout << "CaloTowerStatus::InitRun: Using direct URL override for hot map: " << calibdir_hotMap << std::endl; - m_cdbttree_hotMap = new CDBTTree(calibdir_hotMap); + cdbttree_hotMap = new CDBTTree(calibdir_hotMap); } else { calibdir_hotMap = CDBInterface::instance()->getUrl(m_calibName_hotMap); if (!calibdir_hotMap.empty()) { - m_cdbttree_hotMap = new CDBTTree(calibdir_hotMap); + cdbttree_hotMap = new CDBTTree(calibdir_hotMap); if (Verbosity() > 1) { std::cout << "CaloTowerStatus::Init " << m_detector << " hot map found " << m_calibName_hotMap << " Doing isHot" << std::endl; @@ -148,7 +150,7 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) { std::cout << "CaloTowerStatus::InitRun hot map info, " << m_calibName_hotMap << " not found, not doing isHot" << std::endl; } - } + } } if (Verbosity() > 0) @@ -170,7 +172,10 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) try { CreateNodeTree(topNode); - LoadCalib(); + LoadCalib(cdbttree_chi2, cdbttree_hotMap); + + delete cdbttree_chi2; + delete cdbttree_hotMap; } catch (std::exception &e) { @@ -184,7 +189,7 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) return Fun4AllReturnCodes::EVENT_OK; } -void CaloTowerStatus::LoadCalib() +void CaloTowerStatus::LoadCalib(CDBTTree *cdbttree_chi2, CDBTTree *cdbttree_hotMap) { unsigned int ntowers = m_raw_towers->size(); m_cdbInfo_vec.resize(ntowers); @@ -193,14 +198,14 @@ void CaloTowerStatus::LoadCalib() { unsigned int key = m_raw_towers->encode_key(channel); - if (m_doHotChi2) + if (m_doHotChi2 && cdbttree_chi2) { - m_cdbInfo_vec[channel].fraction_badChi2 = m_cdbttree_chi2->GetFloatValue(key, m_fieldname_chi2); + m_cdbInfo_vec[channel].fraction_badChi2 = cdbttree_chi2->GetFloatValue(key, m_fieldname_chi2); } - if (m_doHotMap) + if (m_doHotMap && cdbttree_hotMap) { - m_cdbInfo_vec[channel].hotMap_val = m_cdbttree_hotMap->GetIntValue(key, m_fieldname_hotMap); - m_cdbInfo_vec[channel].z_score = m_cdbttree_hotMap->GetFloatValue(key, m_fieldname_z_score); + m_cdbInfo_vec[channel].hotMap_val = cdbttree_hotMap->GetIntValue(key, m_fieldname_hotMap); + m_cdbInfo_vec[channel].z_score = cdbttree_hotMap->GetFloatValue(key, m_fieldname_z_score); } } } diff --git a/offline/packages/CaloReco/CaloTowerStatus.h b/offline/packages/CaloReco/CaloTowerStatus.h index 9c261155c4..6ae95a5d0f 100644 --- a/offline/packages/CaloReco/CaloTowerStatus.h +++ b/offline/packages/CaloReco/CaloTowerStatus.h @@ -98,9 +98,6 @@ class CaloTowerStatus : public SubsysReco private: TowerInfoContainer *m_raw_towers{nullptr}; - CDBTTree *m_cdbttree_chi2{nullptr}; - CDBTTree *m_cdbttree_hotMap{nullptr}; - bool m_doHotChi2{true}; bool m_doHotMap{true}; bool m_doAbortNoHotMap{false}; @@ -127,7 +124,7 @@ class CaloTowerStatus : public SubsysReco float z_score_threshold = {5}; float z_score_threshold_default = {5}; - void LoadCalib(); + void LoadCalib(CDBTTree *cdbttree_chi2, CDBTTree *cdbttree_hotMap); struct CDBInfo { From 1c6b4f480350df179d025c890c8f49aeab264f8a Mon Sep 17 00:00:00 2001 From: Chris Pinkenburg Date: Thu, 14 May 2026 14:10:34 -0400 Subject: [PATCH 4/5] cleanup, simplify --- offline/packages/CaloReco/CaloTowerStatus.cc | 71 +++++--------------- offline/packages/CaloReco/CaloTowerStatus.h | 7 +- 2 files changed, 17 insertions(+), 61 deletions(-) diff --git a/offline/packages/CaloReco/CaloTowerStatus.cc b/offline/packages/CaloReco/CaloTowerStatus.cc index 0727ba735d..f9cc5c039d 100644 --- a/offline/packages/CaloReco/CaloTowerStatus.cc +++ b/offline/packages/CaloReco/CaloTowerStatus.cc @@ -8,27 +8,17 @@ #include -#include - #include #include // for SubsysReco #include -#include // for PHIODataNode -#include // for PHNode -#include // for PHNodeIterator -#include // for PHObject #include -#include -#include #include +#include #include -#include // for exit -#include // for exception -#include // for operator<<, basic_ostream -#include // for runtime_error +#include // for operator<<, basic_ostream //____________________________________________________________________________.. CaloTowerStatus::CaloTowerStatus(const std::string &name) @@ -40,20 +30,9 @@ CaloTowerStatus::CaloTowerStatus(const std::string &name) } } -//____________________________________________________________________________.. -CaloTowerStatus::~CaloTowerStatus() -{ - if (Verbosity() > 0) - { - std::cout << "CaloTowerStatus::~CaloTowerStatus() Calling dtor" << std::endl; - } -} - //____________________________________________________________________________.. int CaloTowerStatus::InitRun(PHCompositeNode *topNode) { - PHNodeIterator nodeIter(topNode); - if (m_dettype == CaloTowerDefs::CEMC) { m_detector = "CEMC"; @@ -76,21 +55,19 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) m_detector = "SEPD"; } - CDBTTree *cdbttree_chi2 = nullptr; + CDBTTree *cdbttree_chi2{nullptr}; m_calibName_chi2 = m_detector + "_hotTowers_fracBadChi2"; m_fieldname_chi2 = "fraction"; - std::string calibdir_chi2; if (!m_directURL_chi2.empty()) { - calibdir_chi2 = m_directURL_chi2; - std::cout << "CaloTowerStatus::InitRun: Using direct URL override for chi2: " << calibdir_chi2 << std::endl; - cdbttree_chi2 = new CDBTTree(calibdir_chi2); + std::cout << "CaloTowerStatus::InitRun: Using direct URL override for chi2: " << m_directURL_chi2 << std::endl; + cdbttree_chi2 = new CDBTTree(m_directURL_chi2); } else { - calibdir_chi2 = CDBInterface::instance()->getUrl(m_calibName_chi2); + std::string calibdir_chi2 = CDBInterface::instance()->getUrl(m_calibName_chi2); if (!calibdir_chi2.empty()) { cdbttree_chi2 = new CDBTTree(calibdir_chi2); @@ -155,33 +132,16 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) if (Verbosity() > 0) { - std::cout << "CaloTowerStatus::Init " << m_detector << " doing hotBadChi2=" << std::boolalpha << m_doHotChi2 << " doing hot map=" << std::boolalpha << m_doHotMap << std::endl; + std::cout << "CaloTowerStatus::Init " << m_detector << " doing hotBadChi2=" << std::boolalpha << m_doHotChi2 << " doing hot map=" << std::boolalpha << m_doHotMap << std::endl; } - PHNodeIterator iter(topNode); + LoadCalib(cdbttree_chi2, cdbttree_hotMap); - // Looking for the DST node - PHCompositeNode *dstNode; - dstNode = dynamic_cast(iter.findFirst("PHCompositeNode", "DST")); - if (!dstNode) - { - std::cout << Name() << "::" << m_detector << "::" << __PRETTY_FUNCTION__ - << "DST Node missing, doing nothing." << std::endl; - exit(1); - } - try - { - CreateNodeTree(topNode); - LoadCalib(cdbttree_chi2, cdbttree_hotMap); + delete cdbttree_chi2; + delete cdbttree_hotMap; + + CreateNodeTree(topNode); - delete cdbttree_chi2; - delete cdbttree_hotMap; - } - catch (std::exception &e) - { - std::cout << e.what() << std::endl; - return Fun4AllReturnCodes::ABORTRUN; - } if (Verbosity() > 0) { topNode->print(); @@ -264,7 +224,7 @@ int CaloTowerStatus::process_event(PHCompositeNode * /*topNode*/) m_raw_towers->get_tower_at_channel(channel)->set_isHot(true); } } - if (chi2 > std::min(std::max(badChi2_treshold_const, adc * adc * badChi2_treshold_quadratic),badChi2_treshold_max)) + if (chi2 > std::min(std::max(badChi2_treshold_const, adc * adc * badChi2_treshold_quadratic), badChi2_treshold_max)) { m_raw_towers->get_tower_at_channel(channel)->set_isBadChi2(true); } @@ -283,10 +243,9 @@ void CaloTowerStatus::CreateNodeTree(PHCompositeNode *topNode) if (!m_raw_towers) { std::cout << Name() << "::" << m_detector.c_str() << "::" << __PRETTY_FUNCTION__ - << " " << RawTowerNodeName << " Node missing, doing bail out!" + << " " << RawTowerNodeName << " Node missing, exiting!" << std::endl; - throw std::runtime_error( - "Failed to find " + RawTowerNodeName + " node in CaloTowerStatus::CreateNodes"); + gSystem->Exit(1); } return; diff --git a/offline/packages/CaloReco/CaloTowerStatus.h b/offline/packages/CaloReco/CaloTowerStatus.h index 6ae95a5d0f..21cb652634 100644 --- a/offline/packages/CaloReco/CaloTowerStatus.h +++ b/offline/packages/CaloReco/CaloTowerStatus.h @@ -5,11 +5,8 @@ #include "CaloTowerDefs.h" -#include // for TowerInfoContainer, TowerIn... - #include -#include #include #include @@ -22,7 +19,7 @@ class CaloTowerStatus : public SubsysReco public: CaloTowerStatus(const std::string &name = "CaloTowerStatus"); - ~CaloTowerStatus() override; + ~CaloTowerStatus() override = default; int InitRun(PHCompositeNode *topNode) override; int process_event(PHCompositeNode *topNode) override; @@ -118,7 +115,7 @@ class CaloTowerStatus : public SubsysReco std::string m_directURL_chi2; float badChi2_treshold_const = {1e4}; - float badChi2_treshold_quadratic = {1./100}; + float badChi2_treshold_quadratic = {1. / 100}; float badChi2_treshold_max = {1e8}; float fraction_badChi2_threshold = {0.01}; float z_score_threshold = {5}; From 7fff3252e3eb9012bac12cacd4066f1ba7215e91 Mon Sep 17 00:00:00 2001 From: Chris Pinkenburg Date: Thu, 14 May 2026 14:21:33 -0400 Subject: [PATCH 5/5] need to create node tree earlier --- offline/packages/CaloReco/CaloTowerStatus.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/offline/packages/CaloReco/CaloTowerStatus.cc b/offline/packages/CaloReco/CaloTowerStatus.cc index f9cc5c039d..4cba2f1fcf 100644 --- a/offline/packages/CaloReco/CaloTowerStatus.cc +++ b/offline/packages/CaloReco/CaloTowerStatus.cc @@ -55,6 +55,8 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) m_detector = "SEPD"; } + CreateNodeTree(topNode); + CDBTTree *cdbttree_chi2{nullptr}; m_calibName_chi2 = m_detector + "_hotTowers_fracBadChi2"; @@ -140,8 +142,6 @@ int CaloTowerStatus::InitRun(PHCompositeNode *topNode) delete cdbttree_chi2; delete cdbttree_hotMap; - CreateNodeTree(topNode); - if (Verbosity() > 0) { topNode->print();