Open rly opened 4 years ago
See also related discussion about TTL pulses in #306
It is also worth noting that the schema includes neurodata_types BehavioralEpochs, BehavioralEvents, and BehavioralTimeSeries, which contain any number of IntervalSeries (a TimeSeries), TimeSeries, and TimeSeries objects, respectively. In particular, it seems BehavioralEvents is meant to store irregular event times in a collection of TimeSeries objects.
The need for an Events/LabeledEvents/TTL pulse data type has come up multiple times recently. Here is a summary of possible options to move forward:
Option 1: See #302 for a proposal to add:
Events
type for storing simple events. It has a description and a 1D array of timestamps (unit: 'seconds')LabeledEvents
type that inherits from Events
for storing events that can take on multiple values. Each timestamp is associated with a uint value in the 'label_keys' dataset. Each unique value of the 'label_keys' dataset is associated with a text label in the 'labels' dataset.TTLs
type would inherit from LabeledEvents
. The TTL pulse value would be used as the label key. This type groups all TTL values together in a single dataset, which is often how TTL data come in. If desired, users could separate the TTL events by pulse value and store those data in separate Events
types in /processing/events
.Pros:
Cons:
Units
.Option 2:
LabeledEvents
type that inherits from DynamicTable
. Each row corresponds to a different type of event and has an ID, a text label, a text description, and a 1D array of timestamps. The timestamps dataset would be a ragged array. TTLs
type that inherits from LabeledEvents
.Pros:
Cons:
DynamicTable
data are more complicated and difficult to inspect outside of the API. DynamicTable
is bulky and has historically required hacks to make them work for particular use cases.Option 3: A hybrid between Options 1 and 2.
Events
, LabeledEvents
, and TTLs
types as described in Option 1. These types would live in /acquisition
.AnnotatedEvents
type that inherits from DynamicTable
. Users could separate TTL events by pulse value and store those values in this new table, like in Option 2. Users might want to store only the TTL events and event times that are relevant to their analyses here. This table might live in /processing/events
.Pros:
Cons:
DynamicTable
.Option 4:
Create a subtype of TimeSeries
or BehavioralTimeSeries
or AnnotationSeries
where the 'data' dataset are all ones.
Pros:
Cons:
TimeSeries
has a lot of baggage regarding the data
dataset and uniformly sampled timestamps that are not relevant for event data. Ideally, the type hierarchy would be:
NWBContainer
|-> Events
|-> LabeledEvents
|-> TimeSeries
Since both Events
and TimeSeries
share the timestamps
dataset, but that would be a major breaking change.
I am in favor of option 3. Thoughts, @bendichter @oruebel @ajtritt?
Finally, whichever route we choose, should we make an extension or just build this into the next minor version of NWB? I am in favor of building it in, since we know this will have high usage.
Thanks for really thinking this through and laying it out. I also like 3 and I'd vote for extension because it will allow us to start using this sooner and to vet it before releasing it.
I think developing this as an extension first and then merging in once it has been evaluated is probably the way to go, given that there a few options. The extension can be versioned at 0.x throughout and it can be made explicit that it is not intended for production use, but as an evolving proposal for integration with NWB.
Update: The proposed Events
and LabeledEvents
types will be implemented in an NDX extension in development here: https://github.com/rly/ndx-events
It is often useful to store events as a list of timestamps, without packaging them into a TimeSeries. For example, this list could represent when the animal receives reward, when the animal performs a particular behavior, etc. This is different from TimeIntervals because there is no stop time; these are instantaneous events. Also, TimeIntervals has the baggage of being a DynamicTable. This is simpler than AnnotationSeries which has all the baggage of being a TimeSeries.
@bendichter and I propose two new types for nwb.event.yaml: