ShieldTrigger and StripTrigger #87
Conversation
Need to update m_ShieldPanelGroups for future and test the deadtime.
Updated BGO detector names from 1 back to 0.
- ACS detectors now ACS_X0_0, ACS_X0_1, ACS_X1_0, etc. - GeD detectors now GeD_0, GeD_2 etc.
| double m_ASICDeadTimePerChannel; | ||
|
|
||
| // Shield state tracking | ||
| int m_NumShieldHitCounts; |
There was a problem hiding this comment.
This is for the whole file? It could be very large, do:
unsigned long
|
|
||
| // Shield state tracking | ||
| int m_NumShieldHitCounts; | ||
| int m_ShieldVetoCounter; |
There was a problem hiding this comment.
This is for the whole file? It could be very large, do:
unsigned long
Why not Num / NumberOf in front?
| // Shield state tracking | ||
| int m_NumShieldHitCounts; | ||
| int m_ShieldVetoCounter; | ||
| int m_NumBGOHitsErased; |
There was a problem hiding this comment.
This is for the whole file? It could be very large, do:
unsigned long
| //! Bool to store if ASIC is dead or not | ||
| bool m_IsGeDDead; | ||
|
|
||
| //! Strip deadtime parameters |
| double m_LastTime; | ||
|
|
||
| //! Number of detectors | ||
| static const int nDets = 12; |
| vector<vector<vector<double>>> m_TempEvtTimes; | ||
|
|
||
| //! Stores trigger rates (number of events) for each detector | ||
| vector<int> m_TriggerRates; |
There was a problem hiding this comment.
A number of events is not a rate. Rate is counts per time
|
|
||
| // Read deadtime parameters from file if provided | ||
| if (m_DeadtimeFileName != "" && ParseDeadtimeFile() == false) { | ||
| return false; |
There was a problem hiding this comment.
Check if we shpuld print an error message here
| int countUnique = 0; | ||
|
|
||
| if (CrystalIDs.empty()) { | ||
| return 0.0; |
| cout << m_Name << ": Warning - shield detector " << DetectorID | ||
| << " not found in any panel group" << endl; | ||
| } | ||
| continue; |
| continue; | ||
| } | ||
|
|
||
| // Check deadtime conditions |
There was a problem hiding this comment.
The if's here scare me.
First reverse them:
If EventTime > LastHitTime + ShieldDeadTime
--> new veto window
else if EventTime < LastHitTime + ShieldDelay
--> In coincidence window
else if EventTime < LastHitTime + ShieldDeadTime
--> In dead time
Why do I need the last if? Shouldn't all the remaining cases be in dead time? Did I miss soemthing?
| } | ||
|
|
||
| // Check if event is within veto window | ||
| if (((m_ShieldVetoTime + m_ShieldVetoWindowSize) >= m_EventTime) && |
There was a problem hiding this comment.
Again, can you reverse it, for readability
if eventTime < VetoTime + VetoWindow...
| if (SomeTagNode != 0) { | ||
| m_SomeTagValue = SomeTagNode->GetValue(); | ||
| MXmlNode* DeadtimeFileNode = Node->GetNode("DeadtimeFileName"); | ||
| if (DeadtimeFileNode != 0) { |
| // Initialize the module | ||
|
|
||
| // Read deadtime parameters from file | ||
| if (m_DeadtimeFileName != "" && ParseDeadtimeFile() == false) { |
There was a problem hiding this comment.
I think we need an error message
| for (int ID : ASICChannels) { | ||
| if (ID == 64) { | ||
| if (g_Verbosity >= c_Warning) { | ||
| cout << m_Name << ": Warning - Strip ID is 64; should not happen" << endl; |
There was a problem hiding this comment.
Why not? What about the guard ring which has ID 64
| { | ||
| // Calculate deadtime for GeD ASICs including nearest neighbor readout | ||
| double deadtime = 0; | ||
| int countUnique = 0; |
There was a problem hiding this comment.
That's just a temporary variable, define that below
| int temp_size = ASICChannelsSet.size(); | ||
|
|
||
| if (ID == 64) { | ||
| if (g_Verbosity >= c_Warning) { |
| ASICChannelsSet.insert(ID + 1); | ||
| } | ||
|
|
||
| int new_size = ASICChannelsSet.size(); |
There was a problem hiding this comment.
This might give warnings: you truncate size_t / unsigned long to int
| // Loop through each channel ID | ||
| for (size_t i = 0; i < ASICChannels.size(); i++) { | ||
| int ID = ASICChannels[i]; | ||
| int temp_size = ASICChannelsSet.size(); |
There was a problem hiding this comment.
This should give a warning: you truncate size_t / unsigend long to int
| int h = 0; | ||
| for (int k : ASICChannelsSet) { | ||
| m_EventStripIDs.push_back(k); | ||
| m_EventTimes.push_back(CountTimeVec[h]); |
There was a problem hiding this comment.
Is this named properly, since it is strip related?
m_EventStrip...Time?
| if (!Hit.m_IsGuardRing) { | ||
| int DetID = Hit.m_ROE.GetDetectorID(); | ||
| if (DetID >= 0 && DetID < nDets) { | ||
| xExists[DetID] = 1; |
There was a problem hiding this comment.
Since you just use 0 and 1, shouldn't it be vector ?
| list<MDEEStripHit>& HVHits = Event->GetDEEStripHitHVListReference(); | ||
|
|
||
| // Process all hits and track deadtime | ||
| auto ProcessHits = [&](list<MDEEStripHit>& Hits) { |
There was a problem hiding this comment.
Usually lambda functions should be something short and not ~70 lines of code...
There was a problem hiding this comment.
At a minimum add some explanation what the lambda does
| } | ||
|
|
||
| // Check deadtime status | ||
| if (m_ASICLastHitTime + m_StripsCurrentDeadtime < m_EventTime) { |
There was a problem hiding this comment.
Same as in the above code. Do
If EventTime > ...
check if the last elseif is not just an else
| if (SomeTagNode != 0) { | ||
| m_SomeTagValue = SomeTagNode->GetValue(); | ||
| MXmlNode* DeadtimeFileNode = Node->GetNode("DeadtimeFileName"); | ||
| if (DeadtimeFileNode != 0) { |
2 participants
ShieldTrigger and StripTrigger updated with ASIC and ADC dead time. Changed GeD names to GeD_0, GeD_1 ... and BGO to ACS_X0_0, ACS_X0_1, ACS_X1_0, ACS_X1_1 ...
Still need to add GR vetoes, dead strips, trigger thresholds for both shields and strips and possible telemetry dead time.
ACS energy correction commented out for now. @vfioretti will update and add it back in.