Project

General

Profile

Actions

Development #2262

closed

Development #2256: Demonstrator for digi-based trigger, STS-only

Development #2257: Preparation of software infrastructure

Base class for trigger

Added by Volker Friese 8 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Start date:
09/28/2021
Due date:
10/11/2021
% Done:

100%

Estimated time:
5.00 h

Description

A base class for the trigger is to be implemented. The data interfaces are: CbmTimeslice (input), vector<double> (output). The output is a list of trigger times as input to the event builder.

Target: algo/base/CbmTrigger


Related issues

Blocked by Framework - Development #2261: Data containers for event and time slice digisClosedJan de Cuveland09/28/202110/08/2021

Actions
Blocks Framework - Development #2273: Reconstruction from digi levelAssignedVolker Friese11/08/202111/12/2021

Actions
Precedes Framework - Development #2266: Migrate digi-based trigger algorithmClosedVolker Friese10/12/202110/15/2021

Actions
Precedes Framework - Development #2270: Task for triggeringClosedVolker Friese10/12/202110/22/2021

Actions
Actions #1

Updated by Volker Friese 8 months ago

  • Due date changed from 10/08/2021 to 10/21/2021
  • Start date changed from 09/28/2021 to 10/11/2021
  • Follows Development #2261: Data containers for event and time slice digis added
Actions #2

Updated by Volker Friese 8 months ago

  • Follows deleted (Development #2261: Data containers for event and time slice digis)
Actions #3

Updated by Volker Friese 8 months ago

Actions #4

Updated by Volker Friese 8 months ago

  • Due date changed from 10/21/2021 to 10/11/2021
  • Start date changed from 10/11/2021 to 09/28/2021
Actions #5

Updated by Volker Friese 8 months ago

Actions #6

Updated by Volker Friese 8 months ago

Actions #7

Updated by Volker Friese 8 months ago

Actions #8

Updated by Volker Friese 8 months ago

Actions #9

Updated by Volker Friese 8 months ago

Actions #10

Updated by Volker Friese 8 months ago

  • Status changed from Assigned to In Progress
  • Assignee changed from Volker Friese to Dominik Smith

The trigger task is a very general one - actually it is the real online data processing. For the time being, we have a simple prescription based on the digi time distribution, but the same scheme should also hold for trigger involving partial or complete reconstruction.

What is common for all these cases are the data interfaces:

Input is a collection of collections of digis, so a collection of vector<CbmDigi> with different digi classes. Which detectors / digi classes contribute depends on the setup and on the trigger demands - can be a single detector or all systems at the same time. To give these digi vectors as arguments to the execution method, I propose to have a container class for the digi vectors. By chance (?), this is very similar to what we need as raw event class - one digi vector per system. The only difference is that in the first case, the vector contains all digis in the time slice, in the second case only the digis attributed to the raw event. For the further discussion, see #2261.

Output shall be a vector<double> with the trigger times (relative to the time slice start). What we must discuss is the definition of this time - shall it try to approximate the true event time? Otherwise, it may be particular to a specific trigger. But this does not influence the data structure.

Since there will be a variety of triggers, each with a different set of parameters, it is not well imaginable to pass a parameter container to the execution call in the base class. We will thus need some config step for the concrete algorithms.

Concerning monitoring: on base class level, again the generality does not suggest any specific monitoring data.

That means that the base class will look like (assuming we have a data class CbmTimeslice):

class CbmAlgTrigger: {
public:
vector<double> Exec(const CbmTimeslice& digis)
}

Dominik, can you imagine that e.g. the SeedFinderSlidingWindow would fit to that?

Actions #11

Updated by Dominik Smith 8 months ago

In principle yes. It is pretty similar to the current interface. Right now we use a template, i.e. an input of "const std::vector<inType>*" where "inType" can be a digi class. This we could easily replace with some generic container that contains digis.

The current implementation is even more general however, and also accepts the case of "inType" being "double", i.e. an explicit list of times instead of digis. This is currently used for the case where the digi times of several detectors are combined into a sorted list before running the seed finder. But this is only one possibility. The list of times could in principle be anything, e.g. "hit times" or the result of some more complex reconstruction task. If we force the input to be a collection of digis, we lose this option. In that spirit, an interface of the form

template<class inType>
std::vector<double> Exec(const std::vector<inType>* vIn)

would be less restrictive. Each person implementing a new trigger algorithm can then decide on their own what "inType" should be.

I think the basic point here it that it is really not the digis themselves, but the digi times that are ultimately used by the algorithm. This is true even in the present implementation. If for instance "inType=StsDigi" then the only thing that happens is that the algorithm calls GetTime() on the digis instead of using the content of the vector directly.

Also a small note on nomenclature: My understanding is that "Exec()" is a function which is typically associated with a FairTask, whereas the Algos use something more descriptive. For the unpackers for instance the "Exec()" function of the task calls the "Unpack()" function of the Algo. So perhaps we should also use something like "FindTriggers()" here instead of "Exec()".

Also, another side remark: I was thinking for a while about generic non-templated digi containers and, in absence of a digi base class, had a hard time coming up with something that doesn't involve boost::any or void pointers. Not sure whether this is what we want.

Actions #12

Updated by Volker Friese 8 months ago

Dominik Smith wrote:

If we force the input to be a collection of digis, we lose this option. In that spirit, an interface of the form

template<class inType>
std::vector<double> Exec(const std::vector<inType>* vIn)

would be less restrictive. Each person implementing a new trigger algorithm can then decide on their own what "inType" should be.

Two arguments against this:

1. Exec() must be virtual, otherwise a base class does not make sense. And you cannot have a virtual template function.

2. The syntax restricts the input to a single vector. This is not the most general formulation; you might want to specify several input vectors (digis) to derive a trigger from (e.g.: requiring 20 digis in STS .and. 10 digis in TOF).

would be less restrictive.

I do want to be restrictive. I want the trigger to operate on well-defined input, not on anything.

I think the basic point here it that it is really not the digis themselves, but the digi times that are ultimately used by the algorithm. This is true even in the present implementation. If for instance "inType=StsDigi" then the only thing that happens is that the algorithm calls GetTime() on the digis instead of using the content of the vector directly.

This holds for the digi-based seed finder, but not in general. In complicated triggers involving e.g. track reconstruction, you surely will need more from the digis than just the time information.

My picture follows the very basic path:
1. The unpackers produce vectors of digis (for the moment, one per detector system).
2. One or several of these are input to the trigger, which derives a list of trigger times.
3. The event builder constructs for each trigger an event by moving digis from the input vectors into the event class.

Also a small note on nomenclature: My understanding is that "Exec()" is a function which is typically associated with a FairTask, whereas the Algos use something more descriptive. For the unpackers for instance the "Exec()" function of the task calls the "Unpack()" function of the Algo. So perhaps we should also use something like "FindTriggers()" here instead of "Exec()".

Matter of convention; I am open for anything meaningful.

Also, another side remark: I was thinking for a while about generic non-templated digi containers and, in absence of a digi base class, had a hard time coming up with something that doesn't involve boost::any or void pointers. Not sure whether this is what we want.

I, too. Indeed, I did not see any way around std::any (or boost::any). We have to check with Jan whether the use of this is eligible. It is surely not what he calls "strongly typed".

Actions #13

Updated by Dominik Smith 8 months ago

1. Exec() must be virtual, otherwise a base class does not make sense. And you cannot have a virtual template function.

Ok, point taken. I can envision a purely template-based solution, in which the base class is either also a template or absent entirely, but I understand that this is not what we are doing.

2. The syntax restricts the input to a single vector. This is not the most general formulation; you might want to specify several input vectors (digis) to derive a trigger from (e.g.: requiring 20 digis in STS .and. 10 digis in TOF).

In principle "inType" could also be a vector, i.e.

std::vector<double> Exec(const std::vector<std::vector<double>>* vIn)

which would cover this case if the times are the only thing we need, but for the more general case I agree that a generic container is needed (especially if we want to enforce uniformity of the input).

I, too. Indeed, I did not see any way around std::any (or boost::any). We have to check with Jan whether the use of this is eligible. It is surely not what > he calls "strongly typed".

Ok, I will wait until this is decided.

Actions #14

Updated by Dominik Smith 8 months ago

To get back to the initial question: Yes, the seed finder can surely be adjusted to the new interface.

Actions #15

Updated by Jan de Cuveland 8 months ago

The trigger task is a very general one - actually it is the real online data processing. For the time being, we have a simple prescription based on the digi time distribution, but the same scheme should also hold for trigger involving partial or complete reconstruction.

What is common for all these cases are the data interfaces:

Input is a collection of collections of digis, so a collection of vector<CbmDigi> with different digi classes.

If we define a generic trigger as an algorithm turning digi vectors into time points, this will prevent us from integrating the additional steps of local reconstruction (hit finding, etc.) with the unpacking step in the future. But for future analysis chains, combining all the local reconstruction steps locally may enable significantly better performance due to better cache use and easier parallelization. My feeling is that we should not define a generic trigger base class like this, but rather keep in our structure of the analysis the destinction between local and global algorithms.

Observation: Different trigger algorithms not only operate on different sets of detectors, but also on different types of input data. What we have here is a Digi-based trigger. Many future algorithms will be based on hits/clusters, and so on.

Following the general advice of only generalizing your code if required, my question is: do we immediately foresee a second Digi-based trigger algorithm in addition to the "accumulation point finder" at hand? If so, we could indeed introduce a base class like cbm::alg::DigiTrigger. Otherwise, why don't we just implement the algorithm with the desired interface?

Now, about this desired interface: I understand that the question about std::any stems from the fact that you would like to only look at the timestamp of the digis, and therefore would like to have all digis in a common container - is this right? My feeling about this is that we should do without the common container. Technically, since the time stamp is stored differently in the Digis, we always need a distinction of cases. If we store the digis in a somewhat dynamically typed container, we force the compiler to include this distinction in every single access. If we keep it in different containers, however, we can either work on them independently first (without the need for distinction) and then combine, or we could as well just extract all the timestamps into a new common array as I understand it is currently done.

Then, even though the set of contributing detectors may vary, I feel that we do not need to abstract from the individual detectors. Can't we just put all the detector's Digi-level data into a structure and pass this as the input to the DigiTrigger algorithm? If a detector is foreseen, but not contributing, its data structure could be empty. At our current stage in CBM I feel that the set of detector subsystems is not that dynamic any more.

Lastly, I am still hesitant to subscribe to the notion that a detector's Digi-level data is just a vector of Digis. It may well be possible, but I can envision cases in which additional information would make sense.

All if this combined, it might lead to a structure similar to the following:

struct StsDigis {
  std::vector<StsDigi> digis;
  // ... possibly additional information ...
}

struct TimesliceDigis {
  StsDigis sts;
  TrdDigis trd;
  TofDigis tof;
  // ... more detectors ...
}

class DigiAggregationTrigger {
public:
  std::vector<double> operator()(const TimesliceDigis& ts);
};

What do you think?

Actions #16

Updated by Volker Friese 8 months ago

Jan de Cuveland wrote:

If we define a generic trigger as an algorithm turning digi vectors into time points, this will prevent us from integrating the additional steps of local reconstruction (hit finding, etc.) with the unpacking step in the future. But for future analysis chains, combining all the local reconstruction steps locally may enable significantly better performance due to better cache use and easier parallelization. My feeling is that we should not define a generic trigger base class like this, but rather keep in our structure of the analysis the destinction between local and global algorithms.

Yes, fully right, I already came to a similar conclusion. Deriving the trigger times from the input digis is a very general online task, but this generality should not reflect on the algorithm level, but on the framework level (task / device / whatever). In principle, I think that algorithms should not call other algorithms - the logic of connecting them and their data should be implemented above. So, I agree that if we want a base class for this specific algorithm, it should be like DigiTrigger.

Actions #17

Updated by Volker Friese 8 months ago

Jan de Cuveland wrote:

Following the general advice of only generalizing your code if required, my question is: do we immediately foresee a second Digi-based trigger algorithm in addition to the "accumulation point finder" at hand? If so, we could indeed introduce a base class like cbm::alg::DigiTrigger. Otherwise, why don't we just implement the algorithm with the desired interface?

I think the answer here is "yes". Actually, finding the event "peaks" in the digi stream is not as trivial as it might appear at first, and different implementations are thinkable. For instance, I can already think of three solutions:
  • combining all digi streams into a single one; close an event once the time difference between two subsequent digis exceeds a threshold. This is now implemented in the event builder itself and was used for the beamtime data.
  • for a single detector, accumulate all digis with time difference lower than a threshold w.r.t. the first digi. Trigger an event once the number of digis exceeds a threshold. This is now implemented in the SlidingWindow seed finder.
  • count digis in fixed-size time windows; trigger an event if the number of digis within is above a threshold. This is my preferred idea; the difficulty is that if you have fixed time binning, events may be split between two bins and thus fall below threshold in each of them.

So you see that we will have different implementation to compare and to choose from.

Actions #18

Updated by Volker Friese 8 months ago

Jan de Cuveland wrote:

Now, about this desired interface: I understand that the question about std::any stems from the fact that you would like to only look at the timestamp of the digis, and therefore would like to have all digis in a common container - is this right? My feeling about this is that we should do without the common container. Technically, since the time stamp is stored differently in the Digis, we always need a distinction of cases. If we store the digis in a somewhat dynamically typed container, we force the compiler to include this distinction in every single access. If we keep it in different containers, however, we can either work on them independently first (without the need for distinction) and then combine, or we could as well just extract all the timestamps into a new common array as I understand it is currently done.

Then, even though the set of contributing detectors may vary, I feel that we do not need to abstract from the individual detectors. Can't we just put all the detector's Digi-level data into a structure and pass this as the input to the DigiTrigger algorithm? If a detector is foreseen, but not contributing, its data structure could be empty. At our current stage in CBM I feel that the set of detector subsystems is not that dynamic any more.

Right, and this is what is already done in CbmRawEvent. The discussion with std::any just came about because the hard-coding of the detectors and their digi classes is not very elegant and flexible, but as you point out, we probably do not need the flexibility any longer. In principle, CbmRawEvent can also be used for time-slice digis, just the content will vary. Let's continue this discussion in the respective issue #2261.

Actions #19

Updated by Dominik Smith 8 months ago

Volker Friese wrote:

I think the answer here is "yes". Actually, finding the event "peaks" in the digi stream is not as trivial as it might appear at first, and different implementations are thinkable. For instance, I can already think of three solutions:
  • combining all digi streams into a single one; close an event once the time difference between two subsequent digis exceeds a threshold. This is now implemented in the event builder itself and was used for the beamtime data.
  • for a single detector, accumulate all digis with time difference lower than a threshold w.r.t. the first digi. Trigger an event once the number of digis exceeds a threshold. This is now implemented in the SlidingWindow seed finder.
  • count digis in fixed-size time windows; trigger an event if the number of digis within is above a threshold. This is my preferred idea; the difficulty is that if you have fixed time binning, events may be split between two bins and thus fall below threshold in each of them.

A side remark: The most recent event builder uses the third option, not the first.

Actions #20

Updated by Volker Friese 7 months ago

Dominik Smith wrote:

Volker Friese wrote:

I think the answer here is "yes". Actually, finding the event "peaks" in the digi stream is not as trivial as it might appear at first, and different implementations are thinkable. For instance, I can already think of three solutions:
  • combining all digi streams into a single one; close an event once the time difference between two subsequent digis exceeds a threshold. This is now implemented in the event builder itself and was used for the beamtime data.
  • for a single detector, accumulate all digis with time difference lower than a threshold w.r.t. the first digi. Trigger an event once the number of digis exceeds a threshold. This is now implemented in the SlidingWindow seed finder.
  • count digis in fixed-size time windows; trigger an event if the number of digis within is above a threshold. This is my preferred idea; the difficulty is that if you have fixed time binning, events may be split between two bins and thus fall below threshold in each of them.

A side remark: The most recent event builder uses the third option, not the first.

I think it uses the second one (window size is not fixed, it triggers once a threshold is reached and starts a new window).

Actions #21

Updated by Volker Friese 7 months ago

  • % Done changed from 0 to 10

Following the discussion above, there are some issues to be agreed on before we proceed.

1. I agree that we should generalise only when a second implementation is at hand. Currently, we have the sliding window trigger by Dominik. So, we go without a base class until a second algorithm is developed.

2. The current algorithm uses a single detector (digi array). It is thinkable to use a combinations of detectors, separately evaluated and connected by a logical .and. or .or. Do we need that? If not, the input data for the algorithm should not be CbmDigiTimeslice, but only the detector digi container (e.g., CbmStsDigiData). Otherwise, we would have to tell the algorithm which digi data to use from CbmDigiTimeslice.

3. In view of separation of trigger and event builder, we should make it part of the contract that the delivered trigger times are corrected for time bias w.r.t. the event start, i.e. give the best estimate of the true event times. Only then, the detector-specific time windows used by the event builder can be defined independent from the applied trigger. The time bias of a specific trigger must be obtained a priori, by simulations or other means.

4. I appreciate Dominik's remark that the current trigger actually uses only the time information of the digis. So, it could also be given a vector<uint32_t> or vector<double> as input with this time information. The advantage is that it could then be used, without change, also to other time-stamped data types like clusters, hits or tracks. The disadvantage is that the input vector would have to be prepared by the calling instance, which surely involves a copy process. The other way to introduce generality is to make the execution templated w.r.t. underlying data type, requiring it to have a GetTime() method.

Actions #22

Updated by Volker Friese 7 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 10 to 100

In summary, the desired interface shall be as proposed by Jan:

class DigiTrigger {
public:
std::vector<double> operator()(const TimesliceDigis& ts);
};

Plus the necessary configuration (which detector to trigger on, parameters). I think with that we can go on with a concrete implementation.

Actions

Also available in: Atom PDF