Skip to content

refactor: bco counting code #3996

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

Draft
osbornjd wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

refactor: bco counting code #3996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b5edb0
add to makefile
osbornjd Oct 31, 2025
41a88f5
merge changes into current gl1 and single streaming input
osbornjd Oct 31, 2025
19d5577
Merge branch 'master' of github.com:sPHENIX-Collaboration/coresoftwar…
osbornjd Nov 14, 2025
45a96b5
remove extraneous files
osbornjd Nov 14, 2025
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
4 changes: 2 additions & 2 deletions ...n4AllStreamingLumiCountingInputManager.cc → ...n4AllStreamingLumiCountingInputManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "Fun4AllStreamingLumiCountingInputManager.h"

#include <fun4allraw/InputManagerType.h>
#include "SingleStreamingInputv2.h"
#include "SingleStreamingInput.h"

#include <ffarawobjects/Gl1Packet.h>

Expand Down Expand Up @@ -173,7 +173,7 @@ std::string Fun4AllStreamingLumiCountingInputManager::GetString(const std::strin
return "";
}

void Fun4AllStreamingLumiCountingInputManager::registerStreamingInput(SingleStreamingInputv2 *evtin, InputManagerType::enu_subsystem system)
void Fun4AllStreamingLumiCountingInputManager::registerStreamingInput(SingleStreamingInput *evtin, InputManagerType::enu_subsystem system)
{
evtin->StreamingLumiInputManager(this);
// if the streaming flag is set, we only want the first event from the GL1 to
Expand Down
6 changes: 3 additions & 3 deletions ...un4AllStreamingLumiCountingInputManager.h → ...un4AllStreamingLumiCountingInputManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include <map>
#include <set>
#include <string>
class SingleStreamingInputv2;
class SingleStreamingInput;
class Gl1Packet;
class PHCompositeNode;
class SyncObject;
Expand All @@ -34,7 +34,7 @@ class Fun4AllStreamingLumiCountingInputManager : public Fun4AllInputManager
int SyncIt(const SyncObject *mastersync) override;
int HasSyncObject() const override { return 1; }
std::string GetString(const std::string &what) const override;
void registerStreamingInput(SingleStreamingInputv2 *evtin, InputManagerType::enu_subsystem);
void registerStreamingInput(SingleStreamingInput *evtin, InputManagerType::enu_subsystem);
int FillGl1();
void AddGl1RawHit(uint64_t bclk, Gl1Packet *hit);
void AddGl1Window(uint64_t bco_trim, int negative_window, int positive_window);
Expand Down Expand Up @@ -76,7 +76,7 @@ class Fun4AllStreamingLumiCountingInputManager : public Fun4AllInputManager
bool flat_overflow{false};
uint64_t bco_temp = 0;

std::vector<SingleStreamingInputv2 *> m_Gl1InputVector;
std::vector<SingleStreamingInput *> m_Gl1InputVector;
std::map<uint64_t, Gl1RawHitInfo> m_Gl1RawHitMap;
std::map<uint64_t, std::pair<uint64_t, uint64_t>> m_BCOWindows;
std::map<uint64_t, int> m_BCOBunchNumber;
Expand Down
2 changes: 2 additions & 0 deletions offline/framework/fun4allraw/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pkginclude_HEADERS = \
Fun4AllPrdfOutputManager.h \
Fun4AllRolloverFileOutStream.h \
Fun4AllStreamingInputManager.h \
Fun4AllStreamingLumiCountingInputManager.h \
Fun4AllTriggeredInputManager.h \
intt_pool.h \
InputManagerType.h \
Expand Down Expand Up @@ -75,6 +76,7 @@ libfun4allraw_la_SOURCES = \
Fun4AllPrdfOutputManager.cc \
Fun4AllRolloverFileOutStream.cc \
Fun4AllStreamingInputManager.cc \
Fun4AllStreamingLumiCountingInputManager.cc \
Fun4AllTriggeredInputManager.cc \
intt_pool.cc \
MicromegasBcoMatchingInformation_v1.cc\
Expand Down
50 changes: 50 additions & 0 deletions offline/framework/fun4allraw/SingleGl1PoolInput.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "SingleGl1PoolInput.h"

#include "Fun4AllStreamingInputManager.h"
#include "Fun4AllStreamingLumiCountingInputManager.h"
#include "InputManagerType.h"

#include <ffarawobjects/Gl1Packetv3.h>
Expand Down Expand Up @@ -69,6 +70,25 @@ void SingleGl1PoolInput::FillPool(const unsigned int /*nbclks*/)
{
std::cout << PHWHERE << "Fetching next Event" << evt->getEvtSequence() << std::endl;
}
if ((m_total_event == 0 && evt->getEvtType() == ENDRUNEVENT) ||
(m_total_event != 0 && evt->getEvtSequence() - 2 == m_total_event))
{
m_alldone_flag = true;
m_lastevent_flag = true;
}
if (evt->getEvtSequence() % 5000 == 0)
{
m_alldone_flag = true;
m_lastevent_flag = true;
}
Comment on lines +79 to +83
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The logic here sets the done flags every 5000 events, but this condition will be true multiple times (events 5000, 10000, 15000, etc.). The condition should likely use modulo with a specific check or a different approach. If the intent is to only set flags at specific event intervals, consider using a more precise condition or documenting why flags are set repeatedly.

Suggested change
if (evt->getEvtSequence() % 5000 == 0)
{
m_alldone_flag = true;
m_lastevent_flag = true;
}
// Removed periodic setting of done flags every 5000 events as it is likely incorrect.

Copilot uses AI. Check for mistakes.
if (Verbosity() > 2)
{
if (m_alldone_flag)
{
std::cout << "gl1 all done is true" << std::endl;
}
// else{std::cout<<"gl1 all done is false"<<std::endl;}
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

Commented-out debug code should be removed. If this alternative branch output is needed, consider uncommenting it or removing it entirely to keep the codebase clean.

Suggested change
// else{std::cout<<"gl1 all done is false"<<std::endl;}

Copilot uses AI. Check for mistakes.
}
RunNumber(evt->getRunNumber());
if (GetVerbosity() > 1)
{
Expand Down Expand Up @@ -99,6 +119,15 @@ void SingleGl1PoolInput::FillPool(const unsigned int /*nbclks*/)
{
std::cout << PHWHERE << "Packet 14001 is null ptr" << std::endl;
evt->identify();
m_alldone_flag = true;
m_lastevent_flag = true;
if (StreamingLumiInputManager())
{
StreamingLumiInputManager()->SetEndofEvent(m_alldone_flag, m_lastevent_flag);
StreamingLumiInputManager()->SetEventNumber(EventSequence);
}
m_alldone_flag = false;
m_lastevent_flag = false;
continue;
}
if (Verbosity() > 1)
Expand All @@ -108,6 +137,10 @@ void SingleGl1PoolInput::FillPool(const unsigned int /*nbclks*/)

Gl1Packet *newhit = new Gl1Packetv3();
uint64_t gtm_bco = packet->lValue(0, "BCO");
uint64_t bco_trim = gtm_bco & 0xFFFFFFFFFFU;
m_BCOWindows[bco_trim] = std::make_pair(bco_trim - m_negative_bco_window, bco_trim + m_positive_bco_window);
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

Potential underflow issue: subtracting m_negative_bco_window (default 20) from bco_trim could underflow if bco_trim is less than 20, since both are unsigned integers. This could result in a very large value wrapping around. Consider adding a check or casting to signed integers before subtraction, or ensuring bco_trim is always larger than m_negative_bco_window.

Suggested change
m_BCOWindows[bco_trim] = std::make_pair(bco_trim - m_negative_bco_window, bco_trim + m_positive_bco_window);
uint64_t bco_window_lower = (bco_trim >= m_negative_bco_window) ? (bco_trim - m_negative_bco_window) : 0;
m_BCOWindows[bco_trim] = std::make_pair(bco_window_lower, bco_trim + m_positive_bco_window);

Copilot uses AI. Check for mistakes.
// std::cout<<"BCO "<< m_BCOWindows.begin()->first<<" left "<<m_BCOWindows.begin()->second.first<<" right "<< m_BCOWindows.begin()->second.second<<std::endl;
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

Commented-out debug code should be removed rather than left in the production codebase. If this output is needed for debugging, consider using the Verbosity() mechanism already in place throughout this file.

Suggested change
// std::cout<<"BCO "<< m_BCOWindows.begin()->first<<" left "<<m_BCOWindows.begin()->second.first<<" right "<< m_BCOWindows.begin()->second.second<<std::endl;
if (Verbosity() > 2)
{
std::cout << "BCO " << bco_trim
<< " left " << m_BCOWindows[bco_trim].first
<< " right " << m_BCOWindows[bco_trim].second
<< std::endl;
}

Copilot uses AI. Check for mistakes.
m_BCOBunchNumber[bco_trim] = packet->lValue(0, "BunchNumber");
m_FEEBclkMap.insert(gtm_bco);
newhit->setBCO(packet->lValue(0, "BCO"));
newhit->setHitFormat(packet->getHitFormat());
Expand Down Expand Up @@ -156,6 +189,20 @@ void SingleGl1PoolInput::FillPool(const unsigned int /*nbclks*/)
{
StreamingInputManager()->AddGl1RawHit(gtm_bco, newhit);
}
if (StreamingLumiInputManager())
{
StreamingLumiInputManager()->AddGl1Window(bco_trim, m_negative_bco_window, m_positive_bco_window);
StreamingLumiInputManager()->AddGl1BunchNumber(bco_trim, m_BCOBunchNumber[bco_trim]);
StreamingLumiInputManager()->SetEndofEvent(m_alldone_flag, m_lastevent_flag);
StreamingLumiInputManager()->SetEventNumber(EventSequence);
StreamingLumiInputManager()->SetNegativeWindow(m_negative_bco_window);
StreamingLumiInputManager()->SetPositiveWindow(m_positive_bco_window);
}
if (evt->getEvtSequence() % 5000 == 0)
{
m_alldone_flag = false;
m_lastevent_flag = false;
}
Comment on lines +201 to +205
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The flags are reset here after being potentially set on lines 79-83, but only for events at multiples of 5000. This creates inconsistent behavior where the flags are set at line 79-83 for all events divisible by 5000, then immediately reset at line 201-205 for the same events. The logic for setting and resetting these flags should be clarified and potentially consolidated to avoid this confusing pattern.

Suggested change
if (evt->getEvtSequence() % 5000 == 0)
{
m_alldone_flag = false;
m_lastevent_flag = false;
}
// Removed resetting of m_alldone_flag and m_lastevent_flag here to avoid inconsistent behavior.
// if (evt->getEvtSequence() % 5000 == 0)
// {
// m_alldone_flag = false;
// m_lastevent_flag = false;
// }

Copilot uses AI. Check for mistakes.
m_Gl1RawHitMap[gtm_bco].push_back(newhit);
m_BclkStack.insert(gtm_bco);

Expand Down Expand Up @@ -220,6 +267,9 @@ void SingleGl1PoolInput::CleanupUsedPackets(const uint64_t bclk)
m_FEEBclkMap.erase(iter);
m_BclkStack.erase(iter);
m_Gl1RawHitMap.erase(iter);
auto trimbclk = iter & 0xFFFFFFFFFFU;
m_BCOWindows.erase(trimbclk);
m_BCOBunchNumber.erase(trimbclk);
}
}

Expand Down
12 changes: 10 additions & 2 deletions offline/framework/fun4allraw/SingleGl1PoolInput.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,22 @@ class SingleGl1PoolInput : public SingleStreamingInput
void CreateDSTNode(PHCompositeNode *topNode) override;
void SetBcoRange(const unsigned int i) { m_BcoRange = i; }
// void ConfigureStreamingInputManager() override;

void SetNegativeWindow(const unsigned int value) { m_negative_bco_window = value; }
void SetPositiveWindow(const unsigned int value) { m_positive_bco_window = value; }
void SetTotalEvent(const int value) { m_total_event = value; }
private:
unsigned int m_NumSpecialEvents{0};
unsigned int m_BcoRange{0};

unsigned int m_negative_bco_window = 20;
unsigned int m_positive_bco_window = 325;
Comment on lines +36 to +37
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The member variable names m_negative_bco_window and m_positive_bco_window use snake_case, which is inconsistent with the camelCase convention used for other member variables in this class (e.g., m_NumSpecialEvents, m_BcoRange). Consider renaming to m_NegativeBcoWindow and m_PositiveBcoWindow for consistency.

Copilot uses AI. Check for mistakes.
int m_total_event = std::numeric_limits<int>::max();
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The member variable name m_total_event uses snake_case, which is inconsistent with the camelCase convention used for other member variables in this class (e.g., m_NumSpecialEvents, m_BcoRange). Consider renaming to m_TotalEvent for consistency.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

Missing include for std::numeric_limits. The header uses std::numeric_limits::max() but does not include . This will cause a compilation error. Add #include to the includes section.

Copilot uses AI. Check for mistakes.
bool m_alldone_flag = {false};
bool m_lastevent_flag = {false};
Comment on lines +39 to +40
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

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

The member variable names m_alldone_flag and m_lastevent_flag use snake_case, which is inconsistent with the camelCase convention used for other member variables in this class (e.g., m_NumSpecialEvents, m_BcoRange). Consider renaming to m_AllDoneFlag and m_LastEventFlag for consistency.

Suggested change
bool m_alldone_flag = {false};
bool m_lastevent_flag = {false};
bool m_AllDoneFlag = {false};
bool m_LastEventFlag = {false};

Copilot uses AI. Check for mistakes.
//! map bco to packet
std::map<unsigned int, uint64_t> m_packet_bco;

std::map<uint64_t, std::pair<uint64_t, uint64_t>> m_BCOWindows;
std::map<uint64_t, int> m_BCOBunchNumber;
std::map<uint64_t, std::vector<Gl1Packet *>> m_Gl1RawHitMap;
std::set<uint64_t> m_FEEBclkMap;
std::set<uint64_t> m_BclkStack;
Expand Down
5 changes: 5 additions & 0 deletions offline/framework/fun4allraw/SingleStreamingInput.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
class Eventiterator;
class Fun4AllEvtInputPoolManager;
class Fun4AllStreamingInputManager;
class Fun4AllStreamingLumiCountingInputManager;
class PHCompositeNode;

class SingleStreamingInput : public Fun4AllBase, public InputFileHandler
Expand Down Expand Up @@ -47,6 +48,8 @@ class SingleStreamingInput : public Fun4AllBase, public InputFileHandler
virtual Eventiterator *GetEventiterator() const { return m_EventIterator; }
virtual Fun4AllStreamingInputManager *StreamingInputManager() { return m_StreamingInputMgr; }
virtual void StreamingInputManager(Fun4AllStreamingInputManager *in) { m_StreamingInputMgr = in; }
virtual Fun4AllStreamingLumiCountingInputManager *StreamingLumiInputManager() { return m_StreamingLumiInputMgr; }
virtual void StreamingLumiInputManager(Fun4AllStreamingLumiCountingInputManager *in) { m_StreamingLumiInputMgr = in; }
virtual void CreateDSTNode(PHCompositeNode *) { return; }
virtual void ConfigureStreamingInputManager() { return; }
virtual void SubsystemEnum(const int id) { m_SubsystemEnum = id; }
Expand Down Expand Up @@ -117,6 +120,8 @@ class SingleStreamingInput : public Fun4AllBase, public InputFileHandler
Eventiterator *m_EventIterator{nullptr};
// Fun4AllEvtInputPoolManager *m_InputMgr {nullptr};
Fun4AllStreamingInputManager *m_StreamingInputMgr{nullptr};
Fun4AllStreamingLumiCountingInputManager *m_StreamingLumiInputMgr{nullptr};

uint64_t m_MaxBclkSpread{1000000};
unsigned int m_EventNumberOffset{1}; // packet event counters start at 0 but we start with event number 1
int m_RunNumber{0};
Expand Down
50 changes: 0 additions & 50 deletions offline/framework/rawbcolumi/Makefile.am
View file
Open in desktop

This file was deleted.

Loading