Fix licks, rewards, and stimulus presentations timestamps

[danielsf]

The current BehaviorSession implementation instantiates licks, rewards, and stimulus presentations from the same StimulusTimestamps instance. This is not appropriate. We hav recently learned that licks and rewards should not include monitor delay in their timestamps. Stimulus presentations should

Furthermore, in the case of behavior_ecephys_sessions, the timestamps should be generated by

1. Registering the behavior_ mapping_ and replay_stimulus_files to the sync file.
2. Grabbing only the timestamps that correspond to the behavior_stimulus_file (this can be done with StimulusTimestamps.from_multiple_stimulus_blocks with non-None stim_of_interest)

Tasks

• Add a check in the Licks and Rewards data object constructor to raise an error if not np.isclose(stimulus_timestamps.monitor_delay, 0.0)
• Separate out the data-objects read in from BehaviorSession._read_data_from_stimulus_file into their own independent instantiations
• In the case of a behavior_ecephys_session, get the timestamps for these data objects as described above

[danielsf]

Should be undertaken after #2367 gets merged

[danielsf]

Continuing with my questionable practice of transcribing emails into GitHub tickets, here is an exchange boiling down to "where should we be applying monitor delay, anyway?"

from Corbett:
Hi Scott,

For the VBN data, your assumptions below are correct. The only place we want to apply the monitor delay is the stimulus times. The trial start/stop times are a little trickier, but I would prefer we not apply the delay to these either, since the code logic doesn't.

From: Scott Daniel <scott.daniel@alleninstitute.org>
Sent: Wednesday, April 20, 2022 9:37 AM
To: Corbett Bennett <corbettb@alleninstitute.org>; Marina Garrett marinag@alleninstitute.org>
Subject: Monitor delay

Hi Corbett, Marina,

I’m emailing you because, as Pika makes the final push towards the VBN release, we are trying to make sure that we are treating monitor_delay correctly. I’m emailing you both because a lot of the VBN code is getting recycled from VBO and I want to be aware if your answers to these questions differ (and it is possible that, since the only functional code I have in front of me is VBO code, I may accidentally ask a question that doesn’t make sense in the context of VBN).

High level summary: I think what I have gleaned from the last two months is that monitor_delay should only be applied to stimulus presentation data. Running speed, licks, and rewards timestamps should not have monitor_delay applied to them. Is this true?

Getting down to brass tacks, these are the data structures within the VBN/VBO session objects which this has implications for. I am including my understanding of whether or not monitor_delay should be applied. Please tell me if I am wrong.

running speed table (raw and filtered)– NO monitor_delay

running acquisition table – NO monitor_delay

licks table – NO monitor_delay

rewards table – NO monitor_delay

stimulus presentations table – YES monitor_delay

task parameters table – NA (doesn’t appear to have any timestamps associated with it)

trials table – NO monitor_delay (this is a tricky one; [all of the timestamps in the table] appear to be lick and reward times, but I’m not sure what to do with start_time and stop_time; I assume they also do NOT depend on monitor_delay, but maybe I am wrong)

optotagging table (VBN only) – NO monitor_delay

[danielsf]

Small wrinkle based on Marina's response

Yep. That sounds right to me. Sorry I missed 'change_time' since it wasn't listed in the doc string, but I agree that any stimulus time should get a monitor delay and should correspond to the stim time in the stimulus table.

c
From: Marina Garrett marinag@alleninstitute.org
Sent: Wednesday, April 20, 2022 11:53 AM
To: Corbett Bennett corbettb@alleninstitute.org; Scott Daniel scott.daniel@alleninstitute.org
Subject: Re: Monitor delay

I agree, with one minor quibble re: the trials table. I am fine with the trial start and stop times not having monitor delay applied, because these times are about how the trial structure played out in real time, however I think it might make sense to apply monitor delay to the change_time column because that should represent the actual time that the stimulus changed, which incorporates monitor delay. I also think that the change_time in the trials table should have a corresponding and identical stimulus presentation start_time in the stimulus_presentations table, which has monitor delay applied.

Corbett, thoughts?

I will try to encompass the above decision in this work.

[danielsf]

Corbett confirms the accuracy of this statement

About the trials table:

Given that it seems to be concerned about "did the mouse do what it was expected to when it was expected to", I gather that the trials table is another one of those data objects that only applies to the behavior stimulus block (i.e. there should be no trials table data for mapping or replay).