Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
5adbf5c
djl -- add average calo v2 as an option for flow
Dec 18, 2025
35a6b31
add return if LoadCalibrations failed
Dec 18, 2025
2e9ccc7
djl -- array to vector
Dec 18, 2025
ce0d3af
djl -- change error handling to exit and not check for nullptr
Dec 19, 2025
6ddbed5
Add matching flag to evaluation.
hupereir Apr 14, 2025
3394890
Merge pull request #4089 from hupereir/micromegas_pool_evaluation_update
osbornjd Jan 6, 2026
67423a3
clang-format
osbornjd Jan 6, 2026
014600a
adding quality flag to total energy calc and correlations
Jan 6, 2026
2a654bd
clang-tidy for PHGenFit
pinkenburg Jan 6, 2026
3e94777
appeasing the rabbit
Jan 6, 2026
667a5cc
clang-tidy for mvtx
pinkenburg Jan 6, 2026
45a8c5b
clang-tidy for tpc
pinkenburg Jan 6, 2026
1ba5701
Added sample to hit key. This will allow to use timed hits.
hupereir Dec 16, 2025
815ceef
Add sample to hit key
hupereir Dec 16, 2025
d0a251d
fixed comments
hupereir Dec 16, 2025
0f8adaa
Sort hits by strip number
hupereir Dec 16, 2025
49ff81c
default max sample is 1024 (full range)
hupereir Dec 16, 2025
0c648ef
implemented min and max sample range for selecting clusters.
hupereir Dec 16, 2025
9e7cf18
fix bad bug found by coderabbit
pinkenburg Jan 7, 2026
514cbf6
better performace according to code rabbit
pinkenburg Jan 7, 2026
5c1fb73
make coderabbit happy
pinkenburg Jan 7, 2026
3f30522
fix botched formatting
pinkenburg Jan 7, 2026
0785178
clang-tidy
osbornjd Jan 7, 2026
e06322f
Update offline/packages/micromegas/MicromegasDefs.cc
hupereir Jan 7, 2026
89660ef
Update offline/packages/micromegas/MicromegasDefs.cc
hupereir Jan 7, 2026
ab568eb
Merge pull request #4091 from bseidlit/master
bseidlit Jan 7, 2026
7e9afac
Merge pull request #4090 from osbornjd/clang-format
osbornjd Jan 7, 2026
2f32421
handle max between a double and an int correctly with rounding
pinkenburg Jan 7, 2026
3fe3cd3
CD: New version of silicon pooling
cdean-github Jan 7, 2026
774e982
Merge branch 'master' of github.com:sPHENIX-Collaboration/coresoftwar…
cdean-github Jan 7, 2026
34e66d1
Merge pull request #4092 from pinkenburg/clang-tidy-57
pinkenburg Jan 7, 2026
c6b077c
- removed static as per clang-tidy
hupereir Jan 7, 2026
f5a6ec8
clang-tidy
hupereir Jan 7, 2026
af55b25
add e22 showershape
Shuonli Jan 8, 2026
8d61916
more clang-tidy
hupereir Jan 8, 2026
3a535f0
📝 Add docstrings to `photonclass`
coderabbitai[bot] Jan 8, 2026
16716a5
Merge pull request #4095 from Shuonli/photonclass
pinkenburg Jan 8, 2026
83bb8a6
Merge pull request #4096 from sPHENIX-Collaboration/coderabbitai/docs…
pinkenburg Jan 8, 2026
5ec7de1
Merge pull request #4093 from hupereir/micromegas-decoding-update
osbornjd Jan 8, 2026
15b6eed
Merge pull request #4094 from cdean-github/KFParticle
osbornjd Jan 8, 2026
66f7822
Revert "📝 Add docstrings to `photonclass`"
pinkenburg Jan 8, 2026
ed4562e
Merge pull request #4097 from sPHENIX-Collaboration/revert-4096-coder…
pinkenburg Jan 8, 2026
7e92dc7
Fix bugs in CaloValid to select correct MB + 10 cm trigger
emclaughlin2 Jan 8, 2026
a573ec3
make setters functional
osbornjd Jan 8, 2026
fcb6312
Fix Gl1Packet ID type from string to integer
pinkenburg Jan 8, 2026
c3eb9c5
Merge pull request #4098 from emclaughlin2/master
pinkenburg Jan 8, 2026
b4b37c2
Merge pull request #4099 from osbornjd/cluster_map_name
osbornjd Jan 9, 2026
5628bd2
code rabbit found a bug in the return value
Jan 10, 2026
c7adbfd
Merge branch 'sPHENIX-Collaboration:master' into fixed_calo_v2
danjlis Jan 10, 2026
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
21 changes: 15 additions & 6 deletions offline/QA/Calorimeters/CaloValid.cc
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,16 @@ int CaloValid::process_towers(PHCompositeNode* topNode)
status = status >> 1U; // clang-tidy mark 1 as unsigned
}

totalcemc += offlineenergy;
if (isGood)
{
totalcemc += offlineenergy;
}
Comment on lines +338 to +341
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find the file and check its existence
find . -name "CaloValid.cc" -type f 2>/dev/null

Repository: sPHENIX-Test/coresoftware

Length of output: 107


🏁 Script executed:

# Search for uses of totalcemc, totalihcal, and totalohcal
rg -n -C3 '\b(totalcemc|totalihcal|totalohcal)\b' --type=cpp

Repository: sPHENIX-Test/coresoftware

Length of output: 3381


🏁 Script executed:

# Check git history for recent changes to CaloValid.cc
git log --oneline -20 -- "*CaloValid.cc" 2>/dev/null | head -20

Repository: sPHENIX-Test/coresoftware

Length of output: 141


🏁 Script executed:

# Get the diff of the most recent commit
git show --stat c7adbfd77 2>/dev/null | head -50

Repository: sPHENIX-Test/coresoftware

Length of output: 3015


🏁 Script executed:

# Check if there are uncommitted changes
git diff -- offline/QA/Calorimeters/CaloValid.cc 2>/dev/null | head -100

Repository: sPHENIX-Test/coresoftware

Length of output: 51


🏁 Script executed:

# Look at the broader context around the isGood check to understand the structure
sed -n '310,350p' offline/QA/Calorimeters/CaloValid.cc

Repository: sPHENIX-Test/coresoftware

Length of output: 1579


🏁 Script executed:

# Check the parent commits to understand the branching
git log --oneline --graph -10 -- offline/QA/Calorimeters/CaloValid.cc

Repository: sPHENIX-Test/coresoftware

Length of output: 143


🏁 Script executed:

# Look for the actual PR diff - check if the isGood gating was recently added
git diff HEAD~1 HEAD -- offline/QA/Calorimeters/CaloValid.cc 2>/dev/null | grep -A 10 -B 10 "isGood" | head -100

Repository: sPHENIX-Test/coresoftware

Length of output: 51


🏁 Script executed:

# Check the git log to see if there were changes to CaloValid.cc in recent commits
git log -p -5 --follow -- offline/QA/Calorimeters/CaloValid.cc 2>/dev/null | grep -A 5 -B 5 "isGood.*totalcemc\|totalcemc.*isGood" | head -50

Repository: sPHENIX-Test/coresoftware

Length of output: 51


🏁 Script executed:

# Check if there's a branch comparison we can do
git branch -a 2>/dev/null

Repository: sPHENIX-Test/coresoftware

Length of output: 213


🏁 Script executed:

# Look at the file size and structure to understand better
wc -l offline/QA/Calorimeters/CaloValid.cc

Repository: sPHENIX-Test/coresoftware

Length of output: 110


🏁 Script executed:

# Check if there are any comments in the code indicating recent changes or TODO items
grep -n "TODO\|FIXME\|NOTE\|isGood" offline/QA/Calorimeters/CaloValid.cc | head -30

Repository: sPHENIX-Test/coresoftware

Length of output: 473


🏁 Script executed:

# Compare the current HEAD against master branch to see what changed
git diff master -- offline/QA/Calorimeters/CaloValid.cc 2>/dev/null | grep -A 5 -B 5 "isGood\|totalcemc\|totalihcal\|totalohcal" | head -150

Repository: sPHENIX-Test/coresoftware

Length of output: 2192


🏁 Script executed:

# Check the master branch version of the file to see if isGood gating exists there
git show master:offline/QA/Calorimeters/CaloValid.cc 2>/dev/null | sed -n '330,350p'

Repository: sPHENIX-Test/coresoftware

Length of output: 761


🏁 Script executed:

# Also check lines around IHCAL and OHCAL in master
git show master:offline/QA/Calorimeters/CaloValid.cc 2>/dev/null | sed -n '405,425p'

Repository: sPHENIX-Test/coresoftware

Length of output: 712


Verify intention of gating total energy by tower quality flag.

Total energy accumulation for all three calorimeters (CEMC, IHCAL, OHCAL) is now gated by the isGood flag. This changes the semantics of totalcemc, totalihcal, and totalohcal from accumulating all tower energies to accumulating only "good" tower energies.

This affects the correlation plots at lines 629-632 and the clustering threshold check at line 666. Confirm this behavior change is intentional and verify that downstream analyses depending on these totals remain valid.

🤖 Prompt for AI Agents
In @offline/QA/Calorimeters/CaloValid.cc around lines 338 - 341, The change
gates accumulation of totalcemc/totalihcal/totalohcal by the tower quality flag
isGood, which likely unintentionally changes these totals from "sum of all tower
energies" to "sum of good tower energies"; revert or make explicit: either
remove the isGood conditional when updating totalcemc/totalihcal/totalohcal so
totals always include offlineenergy, or if the new behavior is intended, update
downstream uses (the correlation plots and the clustering threshold check that
reference these totals) to expect "good-only" sums and rename variables to
reflect that (e.g., goodTotalCEMC) and ensure clustering logic uses the correct
total variable; update references to totalcemc/totalihcal/totalohcal in the
correlation plotting and clustering threshold code accordingly.

h_emcaltime->Fill(_timef);
if (offlineenergy > emcal_hit_threshold)
{
h_cemc_etaphi_time->Fill(ieta, iphi, _timef);
h_cemc_etaphi->Fill(ieta, iphi);
if (isGood && (scaledBits[10] || scaledBits[11]))
if (isGood && (scaledBits[10] || scaledBits[12]))
{
h_cemc_etaphi_wQA->Fill(ieta, iphi, offlineenergy);
}
Expand Down Expand Up @@ -407,14 +410,17 @@ int CaloValid::process_towers(PHCompositeNode* topNode)
status = status >> 1U; // clang-tidy mark 1 as unsigned
}

totalihcal += offlineenergy;
if (isGood)
{
totalihcal += offlineenergy;
}
h_ihcaltime->Fill(_timef);

if (offlineenergy > ihcal_hit_threshold)
{
h_ihcal_etaphi->Fill(ieta, iphi);
h_ihcal_etaphi_time->Fill(ieta, iphi, _timef);
if (isGood && (scaledBits[10] || scaledBits[11]))
if (isGood && (scaledBits[10] || scaledBits[12]))
{
h_ihcal_etaphi_wQA->Fill(ieta, iphi, offlineenergy);
}
Expand Down Expand Up @@ -472,14 +478,17 @@ int CaloValid::process_towers(PHCompositeNode* topNode)
status = status >> 1U; // clang-tidy mark 1 as unsigned
}

totalohcal += offlineenergy;
if (isGood)
{
totalohcal += offlineenergy;
}
h_ohcaltime->Fill(_timef);

if (offlineenergy > ohcal_hit_threshold)
{
h_ohcal_etaphi_time->Fill(ieta, iphi, _timef);
h_ohcal_etaphi->Fill(ieta, iphi);
if (isGood && (scaledBits[10] || scaledBits[11]))
if (isGood && (scaledBits[10] || scaledBits[12]))
{
h_ohcal_etaphi_wQA->Fill(ieta, iphi, offlineenergy);
}
Expand Down
2 changes: 1 addition & 1 deletion offline/QA/Calorimeters/CaloValid.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class CaloValid : public SubsysReco

TriggerAnalyzer* trigAna{nullptr};
TH3* h_pi0_trigIB_mass{nullptr};
std::vector<int> triggerIndices{10, 28, 29, 30, 31}; // MBD NS>=1, Photon Triggers
std::vector<int> triggerIndices{10, 12, 28, 29, 30, 31}; // MBD NS>=1, Photon Triggers

TH1* h_cemc_channel_pedestal[128 * 192]{nullptr};
TH1* h_ihcal_channel_pedestal[32 * 48]{nullptr};
Expand Down
10 changes: 10 additions & 0 deletions offline/QA/Tracking/MicromegasClusterQA.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ int MicromegasClusterQA::InitRun(PHCompositeNode* topNode)
<< (m_calibration_filename.empty() ? "unspecified" : m_calibration_filename)
<< std::endl;

std::cout << "MicromegasClusterQA::InitRun - m_sample_min: " << m_sample_min << std::endl;
std::cout << "MicromegasClusterQA::InitRun - m_sample_max: " << m_sample_max << std::endl;

// read calibrations
if (!m_calibration_filename.empty())
{
Expand Down Expand Up @@ -162,6 +165,13 @@ int MicromegasClusterQA::process_event(PHCompositeNode* topNode)
// find associated hits
const auto hit_range = m_cluster_hit_map->getHits(ckey);

// check hit samples
// if none of the associated hits' sample is within acceptable range, skip the cluster
if( std::none_of( hit_range.first, hit_range.second,
[this]( const TrkrClusterHitAssoc::Map::value_type& pair )
{ return MicromegasDefs::getSample( pair.second ) >= m_sample_min && MicromegasDefs::getSample( pair.second ) < m_sample_max; } ) )
{ continue; }

// store cluster size and fill cluster size histogram
const int cluster_size = std::distance(hit_range.first, hit_range.second);
m_h_cluster_size->Fill(detid, cluster_size);
Expand Down
13 changes: 13 additions & 0 deletions offline/QA/Tracking/MicromegasClusterQA.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ class MicromegasClusterQA : public SubsysReco
m_calibration_filename = value;
}

/// set min sample for signal hits
void set_sample_min(uint16_t value) { m_sample_min = value; }

/// set max sample for signal hits
void set_sample_max(uint16_t value) { m_sample_max = value; }


private:
void create_histograms();

Expand Down Expand Up @@ -98,6 +105,12 @@ class MicromegasClusterQA : public SubsysReco
/// keep track of detector names
std::vector<std::string> m_detector_names;

/// min sample for signal
uint16_t m_sample_min = 0;

/// max sample for signal
uint16_t m_sample_max = 1024;

///@name calibration filename
//@{

Expand Down
132 changes: 37 additions & 95 deletions offline/framework/fun4allraw/Fun4AllStreamingInputManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -776,9 +776,15 @@ int Fun4AllStreamingInputManager::FillIntt()
{
h_taggedAllFee_intt->Fill(refbcobitshift);
}
while (m_InttRawHitMap.begin()->first <= select_crossings - m_intt_negative_bco)

for (auto& [bco, hitinfo] : m_InttRawHitMap)
{
for (auto *intthititer : m_InttRawHitMap.begin()->second.InttRawHitVector)
if (bco > select_crossings)
{
break;
}

for (auto *intthititer : hitinfo.InttRawHitVector)
{
if (Verbosity() > 1)
{
Expand All @@ -788,25 +794,9 @@ int Fun4AllStreamingInputManager::FillIntt()
}
inttcont->AddHit(intthititer);
}
for (auto *iter : m_InttInputVector)
{
iter->CleanupUsedPackets(m_InttRawHitMap.begin()->first);
if (m_intt_negative_bco < 2) // triggered mode
{
iter->clearPacketBClkStackMap(m_InttRawHitMap.begin()->first);
iter->clearFeeGTML1BCOMap(m_InttRawHitMap.begin()->first);
}
}
m_InttRawHitMap.begin()->second.InttRawHitVector.clear();
m_InttRawHitMap.erase(m_InttRawHitMap.begin());
if (m_InttRawHitMap.empty())
{
break;
}
}
}
return 0;
}

int Fun4AllStreamingInputManager::FillMvtx()
{
int iret = FillMvtxPool();
Expand Down Expand Up @@ -846,7 +836,7 @@ int Fun4AllStreamingInputManager::FillMvtx()
}
select_crossings += m_RefBCO;

uint64_t ref_bco_minus_range = m_RefBCO < m_mvtx_bco_range ? 0 : m_RefBCO - m_mvtx_bco_range;
uint64_t ref_bco_minus_range = m_RefBCO < m_mvtx_negative_bco ? 0 : m_RefBCO - m_mvtx_negative_bco;
if (Verbosity() > 2)
{
std::cout << "select MVTX crossings"
Expand Down Expand Up @@ -981,90 +971,42 @@ int Fun4AllStreamingInputManager::FillMvtx()
}
taggedPacketsFEEs.clear();

if (m_mvtx_is_triggered)
uint64_t lower_limit = m_mvtx_is_triggered ? select_crossings : select_crossings - m_mvtx_bco_range - m_mvtx_negative_bco;
uint64_t upper_limit = m_mvtx_is_triggered ? select_crossings + m_mvtx_bco_range : select_crossings;

Comment on lines +974 to +976
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential underflow in lower_limit calculation for non-triggered mode.

For non-triggered MVTX, lower_limit is computed as select_crossings - m_mvtx_bco_range - m_mvtx_negative_bco. When expanded, this becomes m_RefBCO - m_mvtx_negative_bco. If m_RefBCO < m_mvtx_negative_bco, the subtraction underflows (wrapping to a very large uint64_t), causing the loop at line 977 to skip all entries via the continue at line 981.

While line 839 guards against this scenario by capping ref_bco_minus_range at 0, the lower_limit calculation here does not have equivalent protection.

🛡️ Add underflow protection
-  uint64_t lower_limit = m_mvtx_is_triggered ? select_crossings : select_crossings - m_mvtx_bco_range - m_mvtx_negative_bco;
+  uint64_t lower_limit = m_mvtx_is_triggered 
+      ? select_crossings 
+      : (select_crossings > m_mvtx_bco_range + m_mvtx_negative_bco 
+          ? select_crossings - m_mvtx_bco_range - m_mvtx_negative_bco 
+          : 0);
   uint64_t upper_limit = m_mvtx_is_triggered ? select_crossings + m_mvtx_bco_range : select_crossings;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uint64_t lower_limit = m_mvtx_is_triggered ? select_crossings : select_crossings - m_mvtx_bco_range - m_mvtx_negative_bco;
uint64_t upper_limit = m_mvtx_is_triggered ? select_crossings + m_mvtx_bco_range : select_crossings;
uint64_t lower_limit = m_mvtx_is_triggered
? select_crossings
: (select_crossings > m_mvtx_bco_range + m_mvtx_negative_bco
? select_crossings - m_mvtx_bco_range - m_mvtx_negative_bco
: 0);
uint64_t upper_limit = m_mvtx_is_triggered ? select_crossings + m_mvtx_bco_range : select_crossings;
🤖 Prompt for AI Agents
In @offline/framework/fun4allraw/Fun4AllStreamingInputManager.cc around lines
974 - 976, The computation of lower_limit in Fun4AllStreamingInputManager.cc can
underflow when m_mvtx_is_triggered is false because select_crossings -
m_mvtx_bco_range - m_mvtx_negative_bco is done on uint64_t; change the
non-triggered branch to clamp at 0: when m_mvtx_is_triggered is false, check if
select_crossings <= (m_mvtx_bco_range + m_mvtx_negative_bco) and set lower_limit
= 0, otherwise set lower_limit = select_crossings - m_mvtx_bco_range -
m_mvtx_negative_bco; leave the triggered branch and upper_limit logic as-is to
avoid wrapping.

for (auto& [bco, hitinfo] : m_MvtxRawHitMap)
{
while (select_crossings <= m_MvtxRawHitMap.begin()->first && m_MvtxRawHitMap.begin()->first <= select_crossings + m_mvtx_bco_range) // triggered
if (bco < lower_limit)
{
if (Verbosity() > 2)
{
std::cout << "Adding 0x" << std::hex << m_MvtxRawHitMap.begin()->first
<< " ref: 0x" << select_crossings << std::dec << std::endl;
}
for (auto *mvtxFeeIdInfo : m_MvtxRawHitMap.begin()->second.MvtxFeeIdInfoVector)
{
if (Verbosity() > 1)
{
mvtxFeeIdInfo->identify();
}
mvtxEvtHeader->AddFeeIdInfo(mvtxFeeIdInfo);
delete mvtxFeeIdInfo;
}
m_MvtxRawHitMap.begin()->second.MvtxFeeIdInfoVector.clear();
mvtxEvtHeader->AddL1Trg(m_MvtxRawHitMap.begin()->second.MvtxL1TrgBco);
continue;
}
if (bco > upper_limit)
{
break;
}

for (auto *mvtxhititer : m_MvtxRawHitMap.begin()->second.MvtxRawHitVector)
{
if (Verbosity() > 1)
{
mvtxhititer->identify();
}
mvtxcont->AddHit(mvtxhititer);
}
for (auto *iter : m_MvtxInputVector)
{
iter->CleanupUsedPackets(m_MvtxRawHitMap.begin()->first);
}
m_MvtxRawHitMap.begin()->second.MvtxRawHitVector.clear();
m_MvtxRawHitMap.begin()->second.MvtxL1TrgBco.clear();
m_MvtxRawHitMap.erase(m_MvtxRawHitMap.begin());
// m_MvtxRawHitMap.empty() need to be checked here since we do not call FillPoolMvtx()
if (m_MvtxRawHitMap.empty())
{
break;
}
if (Verbosity() > 2)
{
std::cout << "Adding 0x" << std::hex << bco
<< " ref: 0x" << select_crossings << std::dec << std::endl;
}
}
else
{
while (select_crossings - m_mvtx_bco_range - m_mvtx_negative_bco <= m_MvtxRawHitMap.begin()->first && m_MvtxRawHitMap.begin()->first <= select_crossings) // streamed
for (auto *mvtxFeeIdInfo : hitinfo.MvtxFeeIdInfoVector)
{
if (Verbosity() > 2)
{
std::cout << "Adding 0x" << std::hex << m_MvtxRawHitMap.begin()->first
<< " ref: 0x" << select_crossings << std::dec << std::endl;
}
for (auto *mvtxFeeIdInfo : m_MvtxRawHitMap.begin()->second.MvtxFeeIdInfoVector)
if (Verbosity() > 1)
{
if (Verbosity() > 1)
{
mvtxFeeIdInfo->identify();
}
mvtxEvtHeader->AddFeeIdInfo(mvtxFeeIdInfo);
delete mvtxFeeIdInfo;
mvtxFeeIdInfo->identify();
}
m_MvtxRawHitMap.begin()->second.MvtxFeeIdInfoVector.clear();
mvtxEvtHeader->AddL1Trg(m_MvtxRawHitMap.begin()->second.MvtxL1TrgBco);
mvtxEvtHeader->AddFeeIdInfo(mvtxFeeIdInfo);
}
mvtxEvtHeader->AddL1Trg(hitinfo.MvtxL1TrgBco);

for (auto *mvtxhititer : m_MvtxRawHitMap.begin()->second.MvtxRawHitVector)
{
if (Verbosity() > 1)
{
mvtxhititer->identify();
}
mvtxcont->AddHit(mvtxhititer);
}
for (auto *iter : m_MvtxInputVector)
{
iter->CleanupUsedPackets(m_MvtxRawHitMap.begin()->first);
}
m_MvtxRawHitMap.begin()->second.MvtxRawHitVector.clear();
m_MvtxRawHitMap.begin()->second.MvtxL1TrgBco.clear();
m_MvtxRawHitMap.erase(m_MvtxRawHitMap.begin());
// m_MvtxRawHitMap.empty() need to be checked here since we do not call FillPoolMvtx()
if (m_MvtxRawHitMap.empty())
for (auto *mvtxhititer : hitinfo.MvtxRawHitVector)
{
if (Verbosity() > 1)
{
break;
mvtxhititer->identify();
}
mvtxcont->AddHit(mvtxhititer);
}
}

Expand Down Expand Up @@ -1422,7 +1364,7 @@ int Fun4AllStreamingInputManager::FillMicromegasPool()

int Fun4AllStreamingInputManager::FillMvtxPool()
{
uint64_t ref_bco_minus_range = m_RefBCO < m_mvtx_bco_range ? m_mvtx_bco_range : m_RefBCO - m_mvtx_bco_range;
uint64_t ref_bco_minus_range = m_RefBCO < m_mvtx_negative_bco ? m_mvtx_negative_bco : m_RefBCO - m_mvtx_negative_bco;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Inconsistent underflow handling - returns wrong value.

The ternary expression returns m_mvtx_negative_bco when m_RefBCO < m_mvtx_negative_bco, which doesn't prevent underflow and returns a semantically incorrect value. Line 839 correctly returns 0 for the same condition, and FillInttPool (lines 1243-1247) uses a clearer if-statement pattern.

🐛 Fix to match line 839 pattern
-  uint64_t ref_bco_minus_range = m_RefBCO < m_mvtx_negative_bco ? m_mvtx_negative_bco : m_RefBCO - m_mvtx_negative_bco;
+  uint64_t ref_bco_minus_range = m_RefBCO < m_mvtx_negative_bco ? 0 : m_RefBCO - m_mvtx_negative_bco;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uint64_t ref_bco_minus_range = m_RefBCO < m_mvtx_negative_bco ? m_mvtx_negative_bco : m_RefBCO - m_mvtx_negative_bco;
uint64_t ref_bco_minus_range = m_RefBCO < m_mvtx_negative_bco ? 0 : m_RefBCO - m_mvtx_negative_bco;
🤖 Prompt for AI Agents
In @offline/framework/fun4allraw/Fun4AllStreamingInputManager.cc at line 1367,
The ternary initializing ref_bco_minus_range is wrong: it returns
m_mvtx_negative_bco when m_RefBCO < m_mvtx_negative_bco, which allows underflow
and is semantically incorrect; change the expression to return 0 in that case
and m_RefBCO - m_mvtx_negative_bco otherwise (i.e., mirror the check used at
line 839 and the if-style pattern used in FillInttPool). Locate the
initialization of ref_bco_minus_range in
Fun4AllStreamingInputManager::(constructor or method where it appears) and
replace the ternary so underflow is prevented by returning 0 when m_RefBCO <
m_mvtx_negative_bco.

for (auto *iter : m_MvtxInputVector)
{
if (Verbosity() > 3)
Expand Down
51 changes: 29 additions & 22 deletions offline/framework/fun4allraw/SingleMicromegasPoolInput_v2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ void SingleMicromegasPoolInput_v2::createQAHistos()
m_evaluation_file.reset(new TFile(m_evaluation_filename.c_str(), "RECREATE"));
m_evaluation_tree = new TTree("T", "T");
m_evaluation_tree->Branch("is_heartbeat", &m_waveform.is_heartbeat);
m_evaluation_tree->Branch("matched", &m_waveform.matched);
m_evaluation_tree->Branch("packet_id", &m_waveform.packet_id);
m_evaluation_tree->Branch("fee_id", &m_waveform.fee_id);
m_evaluation_tree->Branch("channel", &m_waveform.channel);
Expand Down Expand Up @@ -912,6 +913,13 @@ void SingleMicromegasPoolInput_v2::process_fee_data(int packet_id, unsigned int

// try get gtm bco matching fee
const auto& fee_bco = payload.bx_timestamp;
if( m_do_evaluation )
{
m_waveform.is_heartbeat = is_heartbeat;
m_waveform.fee_id = fee_id;
m_waveform.channel = payload.channel;
m_waveform.fee_bco = fee_bco;
}

// find matching gtm bco
uint64_t gtm_bco = 0;
Expand All @@ -920,9 +928,20 @@ void SingleMicromegasPoolInput_v2::process_fee_data(int packet_id, unsigned int
{
// assign gtm bco
gtm_bco = result.value();
}
else
{
if( m_do_evaluation )
{
m_waveform.matched = true;
m_waveform.gtm_bco_matched = gtm_bco;
{
const auto predicted = bco_matching_information.get_predicted_fee_bco(gtm_bco);;
if( predicted )
{
m_waveform.fee_bco_predicted_matched = predicted.value();
}
}
m_evaluation_tree->Fill();
}
} else {
// increment counter and histogram
++m_waveform_counters[packet_id].dropped_bco;
++m_fee_waveform_counters[fee_id].dropped_bco;
Expand All @@ -935,28 +954,16 @@ void SingleMicromegasPoolInput_v2::process_fee_data(int packet_id, unsigned int
++m_fee_heartbeat_counters[fee_id].dropped_bco;
}

// skip the waverform
continue;
}

if (m_do_evaluation)
{
m_waveform.is_heartbeat = (payload.type == HEARTBEAT_T);
m_waveform.fee_id = fee_id;
m_waveform.channel = payload.channel;
m_waveform.fee_bco = fee_bco;

m_waveform.gtm_bco_matched = gtm_bco;
if( m_do_evaluation )
{
const auto predicted = bco_matching_information.get_predicted_fee_bco(gtm_bco);
;
if (predicted)
{
m_waveform.fee_bco_predicted_matched = predicted.value();
}
m_waveform.matched = false;
m_waveform.gtm_bco_matched = 0;
m_waveform.fee_bco_predicted_matched = 0;
m_evaluation_tree->Fill();
}

m_evaluation_tree->Fill();
// skip the waverform
continue;
}

// ignore heartbeat waveforms
Expand Down
3 changes: 3 additions & 0 deletions offline/framework/fun4allraw/SingleMicromegasPoolInput_v2.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ class SingleMicromegasPoolInput_v2 : public SingleStreamingInput
/// true if measurement is hearbeat
bool is_heartbeat = false;

/// true if matched
bool matched = false;

/// ll1 bco
uint64_t gtm_bco_first {0};

Expand Down
Loading