Event builder device
Implemented Fair MQ device and calling script for algo::EventBuilder.
The good news is that CbmDeviceEventBuilder has been implemented and probably does what it should. The bad news is, that on real data, the new algorithm runs much much slower than the former implementation "CbmAlgoBuildRawEvents".
The most likely reason for this is that the original implementation uses smart iterators, which store the final position within a digi vector after completing an event. The new implementation re-obtains the boundaries of the time window for each new trigger using std::lower_bound and std::upper_bound, which amounts to looping through the entire digi vector again. For the small samples usually considered in simulations this is apparently negligible, but for real data, which has orders of magnitude more events per timeslice, this becomes a huge issue.
Most likely this can be fixed however. It should not be too hard to store the final position of the iterator also in the new class.
Another problem with the current state of the code, is that the "DigiEventSink" crashes with a segfault. This is under investigation, and is why I have marked this MR as WIP.
This MR is based on !668 (closed) and includes all of those commits.
Merge request reports
Activity
yes, in your case you can probably either add an option to directly sink to a root file withing your device, or make a sink device working on your output stream of CbmDigiEvent objects (which could then be run in // to a consumer device).
In both cases, you should remove the
FairMQParts& partsIn
in the SendEvents method, as it was there only to attach the full ts with the reference only CbmEventIf I write the events directly to the output tree, I wouldn't even need to serialize them, right?
But I wonder whether I should keep a separate event sink, regardless. It seems like it does more stuff than just writing the events to file. Like handle missed timeslices for instance.
If I write the events directly to the output tree, I wouldn't even need to serialize them, right?
I think you would still need to have a serialized output to account for eventual "online consumers". But that could be debugged in a second phase
But I wonder whether I should keep a separate event sink, regardless. It seems like it does more stuff than just writing the events to file. Like handle missed timeslices for instance.
If you want to have multiple event builder working in parallel and a single output file, you will indeed need a sink device to gather the events, optionally re-order them and only then write to the common file
added 13 commits
-
9e48083b...4714aae4 - 6 commits from branch
computing:master
- 39862176 - Generalized CbmTaskTriggerDigi from STS to all detectors.
- d76d0c88 - Generalized algo::EventBuilder and its task to detectors other than STS.
- 8e34f4d4 - Generalized diagnostic output in CbmTaskBuildEvents.
- d9340e32 - Updated _GTestEventBuilder to account for multiple detectors.
- abcd590e - Started to implement MQ device for algo::EventBuilder.
- 6e5fef2a - CbmDeviceBuildRawEvents: Switched from root serializer to boost serializer.
- e9e3ab22 - CbmDeviceEventBuilder: CbmDigiEvent vector is now written to output tree.
Toggle commit list-
9e48083b...4714aae4 - 6 commits from branch
- Resolved by Dominik Smith
@p.-a.loizeau My latest commit is supposed to write the event vector to a root file. It follows closely what is done in CbmDeviceDigiEventSink.cxx but produces a seg fault. Could you please take a quick look and tell me whether there is something obvious that I am doing wrong here?
Best would be to just look at the diff between the last commit and the second to last.
Thanks.
- Resolved by Dominik Smith
added 40 commits
-
e9e3ab22...2a21eab2 - 33 commits from branch
computing:master
- 90d120d1 - Generalized CbmTaskTriggerDigi from STS to all detectors.
- 8536a06c - Generalized algo::EventBuilder and its task to detectors other than STS.
- e245b6c5 - Generalized diagnostic output in CbmTaskBuildEvents.
- 51322fb5 - Updated _GTestEventBuilder to account for multiple detectors.
- 2aba3dfe - Started to implement MQ device for algo::EventBuilder.
- ec9d4db0 - CbmDeviceBuildRawEvents: Switched from root serializer to boost serializer.
- d3fbcf68 - CbmDeviceEventBuilder: CbmDigiEvent vector is now written to output tree.
Toggle commit list-
e9e3ab22...2a21eab2 - 33 commits from branch
@p.-a.loizeau Ok, this version works now. I just noticed your "FIXME" in the event sink class, which refers to the same issue with event headers. I copied it over.
I left some of the bool flags related to histograms in the class. They might be needed later.
I'm leaving this MR as WIP, as I still want to do the following:
- Make the writing to file switchable. We might want to either use the event sink or write directly to file.
- Make a proper event sink for this class.
Edited by Dominik Smith- Resolved by Dominik Smith
@p.-a.loizeau I have been looking at CbmDeviceDigiEventSink, and one thing I have noticed is that it includes some functions to deal with out-of-order timeslices, that rely on the presence of the timeslice metadata in the input channel. In principle, do we want to keep this? If so, then I would also need to send the TS metadata to the out channel in the eventbuilder, right?
From what I can tell, the out-of-order function doesn't need the raw TS data. Only events and metadata are sufficient, right?
added 21 commits
-
d3fbcf68...451396eb - 13 commits from branch
computing:master
- 54c152b7 - Generalized CbmTaskTriggerDigi from STS to all detectors.
- 8e63c48c - Generalized algo::EventBuilder and its task to detectors other than STS.
- 17c26ec8 - Generalized diagnostic output in CbmTaskBuildEvents.
- c9fb9614 - Updated _GTestEventBuilder to account for multiple detectors.
- cd278d40 - Started to implement MQ device for algo::EventBuilder.
- 1f3cd2e9 - CbmDeviceBuildRawEvents: Switched from root serializer to boost serializer.
- 3990e5ea - CbmDeviceEventBuilder: CbmDigiEvent vector is now written to output tree.
- f5f78889 - CbmDeviceEventBuilder: Can send, receive and write TS metadata.
Toggle commit list-
d3fbcf68...451396eb - 13 commits from branch
Most likely this can be fixed however. It should not be too hard to store the final position of the iterator also in the new class.
This is straightforward if overlap of events is excluded - then, you can start from the pointer to the end of the previous event. However, our decision is to allow overlaps, and then it is not easy to see how the start of the search for the new events can be a priori determined. I will look how you do that in the old algorithm.
What I did for all "scanners" (time checker, event builder, ...) was storing the first position of the iterator in the current event/test window: if the input is time sorted and the event windows are identical (single trigger type per algo instance), this covers the worst case which would be a fully overlaping event starting on the same digi (e.g. if interval between the two seeds smaller than the typical inter-digi interval/precision).
For small event sizes (relative to buffer scanned), reasonable event frequency and reasonable fraction of overlapping events, this goes toward a
2n
looping instead of the brute forcen * nb events
looping.Something like:
Evt n : ...........S--------E| Evt n + 1: .....S--------E| Evt n + 3: S-----------E| Evt n + 4: ...................S---E| Evt n + 5: ...................S---E|
@p.-a.loizeau I have a question about how TS metadata is treated in CbmDeviceDigiEventSink.cxx. The question is best phrased as a comparison with how the event header is treated. In the event sink class, I encounter the following type of construction:
In the "Init()" function:
fTimeSliceMetaDataArray = new TClonesArray("TimesliceMetaData", 1); fpFairRootMgr->Register("TimesliceMetaData", "TS Meta Data", fTimeSliceMetaDataArray, kTRUE); fEvtHeader = new CbmTsEventHeader(); fpFairRootMgr->Register("EventHeader.", "Event", fEvtHeader, kTRUE);
Then in a sub-function of "HandleData()":
(*fEvtHeader) = std::move(unpTs.fCbmTsEventHeader); new ((*fTimeSliceMetaDataArray)[fTimeSliceMetaDataArray->GetEntriesFast()]) TimesliceMetaData(std::move(unpTs.fTsMetaData));
The way interpret this, is that CbmTsEventHeader objects can be written into the file sink directly, while TS meta data must be stored in a TClonesArray.
I may be wrong, but it looks a lot as if "fTimeSliceMetaDataArray" only ever contains one element. It is slightly involved to see this, as this array gets used also in the processing of the queue of missed time slices, but the way I see it, each time a new element is added, there is a subsequent calling of
fTimeSliceMetaDataArray->Clear();
So my question is, is there a reason we need to use TClonesArray for this. Can't I just write the object to file, the same way as with the event header?
I should add, that the event header receives a special treatment when writing to file. It is written by a
fpFairRootMgr->FillEventHeader(fEvtHeader);
statement, whereas other fields that have been "registered" are written byfpFairRootMgr->Fill();
. A better comparison is therefore perhaps the event vector. Here we essentially have the same thing: We write exactly one event vector for each time slice, and reset it at the end.added 11 commits
-
f5f78889...865362c7 - 2 commits from branch
computing:master
- c19962de - Generalized CbmTaskTriggerDigi from STS to all detectors.
- 40948304 - Generalized algo::EventBuilder and its task to detectors other than STS.
- 07b96ae9 - Generalized diagnostic output in CbmTaskBuildEvents.
- 1af788f5 - Updated _GTestEventBuilder to account for multiple detectors.
- 588fe76d - Started to implement MQ device for algo::EventBuilder.
- 9fd7d67c - CbmDeviceBuildRawEvents: Switched from root serializer to boost serializer.
- 18def991 - CbmDeviceEventBuilder: CbmDigiEvent vector is now written to output tree.
- 1d59cb92 - CbmDeviceEventBuilder: Can send, receive and write TS metadata.
- 93214b77 - Cleaned up CbmDeviceEventBuilder. Made writing to file switchable.
Toggle commit list-
f5f78889...865362c7 - 2 commits from branch
I do not remember completely, but I think it was done for backward compatibility reasons with the already produced "non-selected unpacked data" files and the analysis based on them. At least all occurrences of
TimesliceMetaData
in the foldersMQ
,fles
andreco
are using the TClonesArray.But indeed, getting rid of the container would be nice (and probably allow to remove the TObject dependency)
Ok, in the current version I left the TClonesArray in, but it should now be fairly easy to remove if needed. I also removed some more global fields. The event header, TS meta data and event vector are now only stored locally for the purpose of de-/serialization. They are only copied to global objects when they are send to the FairRootManager for writing to file.
On a related note, I think I now understand the origin of the original seg fault. It is simply that FairRootManager likes to own all the objects that it deals with. So if you try to create another pointer that points to the same object, you get in trouble. Unfortunately this means we need multiple copies if we want to serialize a FairMQ message and write to file at the same time, since the serializers also want pointers. I guess this was the meaning of your original "FIXME" message.
I also found a "FairRunOnline" object in the header and Init() function that doesn't seem to be used anywhere. I'm currently testing whether it can be removed.