Introduced monitoring for cbm::algo::EventBuilder.
Monitoring data is included in cbm::algo::EventBuilder.
In keeping with the style of other algorithms, a struct with monitoring data is introduced, and a special "resultType" is introduced which combines the regular output with the monitoring information.
Accordingly, GTestEventBuilder was modified to extend the test coverage to the monitoring data. In passing the TRD support for this test was fixed.
Overall, I'm not super happy with the aesthetics of this code. The added blocks of the form
monitor.fSts.fNum += ts.fData.fSts.fDigis.size();
monitor.fRich.fNum += ts.fData.fRich.fDigis.size();
monitor.fMuch.fNum += ts.fData.fMuch.fDigis.size();
...
seem a bit cluttery. The only solution that comes to my mind however, would be to use a std::map<ECbmModuleId,EventBuilderDetectorMonitorData>
inside the monitor struct. I guess this is not an option, due to the requirement of the monitoring data to be light-weight.
Another issue is that, in places where one only wants to access the constructed events, but not the monitoring data, the most compact expression is something like
*fEvents = fAlgo(*fTimeslice, *fTriggers).first;
which is a bit contrived.
I chose to put the monitoring incrementation outside of the switch(ECbmModuleId) statement, again for aesthetic reasons. This can in principle lead to some unnecessary "increment by zero" operations. These will be negligible against "CopyRange()" however, and might even be optimized away by the compiler.
What is nice, is that the output could immediately be verified, by using the existing GTest.
Merge request reports
Activity
requested review from @v.friese
assigned to @v.friese
Dear @f.uhlig, @v.friese, @p.-a.loizeau,
you have been identified as code owner of at least one file which was changed with this merge request.
Please check the changes and approve them or request changes.
added CodeOwners label
mentioned in merge request !1209 (merged)
The reason why there was no monitoring for the event building before is that the algorithm is so simple that it is indeed hard to find something that one can monitor. What you do here is just count the number of input and output data. This is information that can be accessed just looking at the respective data and thus does not need to be included in the algorithm itself. However, the information itself is valuable. Probably, it is too much overhead to introduce a separate software piece just doing this gymnastics.
What I would also find interesting and worthwhile monitoring is the number of digis which are duplicated because of the event overlap. This is information which would be hard to get outside of the algorithm. Can you imagine an easy way to implement that without blowing up this simple algorithm unduly?
Yes, I agree that it is hard to find good monitoring targets.
If the information gathered in this MR is useful at runtime at all, then we might as well gather it directly during event building. Otherwise we need yet another class to do the same, possibly with more overhead.
Hmm. From the top of my head, to count the number of duplicated digis, I would do the following:
- Create a vector of bools of the same size as the input vector. Initialize the entries to false.
- Each time a digi is added to an event, check if the corresponding bool is true. If yes, then increment a counter. If not, set to true.
This will certainly produce a performance hit, possibly a big one. On the other hand, here is the only place where this information is available.
Alternatively: Create a list of the indices of the digis which were used. Then check for duplicates for each entry. This is at least an O(N^2) operation. Probably the first variant is better.
added 27 commits
-
b5d6ee2d...15a2da7b - 26 commits from branch
computing:master
- b35d3e3e - Introduced monitoring for cbm::algo::EventBuilder.
-
b5d6ee2d...15a2da7b - 26 commits from branch
Overall, I'm not super happy with the aesthetics of this code. The added blocks of the form
monitor.fSts.fNum += ts.fData.fSts.fDigis.size(); monitor.fRich.fNum += ts.fData.fRich.fDigis.size(); monitor.fMuch.fNum += ts.fData.fMuch.fDigis.size(); ...
seem a bit cluttery. The only solution that comes to my mind however, would be to use a
std::map<ECbmModuleId,EventBuilderDetectorMonitorData>
inside the monitor struct. I guess this is not an option, due to the requirement of the monitoring data to be light-weight.The way to avoid this would be to pass the monitor structure to the CopyRange() method, in which it would be updated. We will leave this for the future.
mentioned in merge request !1230 (closed)