ticket/nnnn/sync/stim/aligner

[danielsf]

This PR ultimately adds the utility function

allensdk.brain_observatory.sync_stim_aligner.get_start_frames_from_stimulus_blocks

which is needed to scan a single sync file and find the indices of the starting frames for the behavior_, mapping_, and replay_ stimulus blocks from a single sync file. It is, ultimately, a re-implementation of the method here

https://github.com/AllenInstitute/ecephys_etl_pipelines/blob/main/src/ecephys_etl/modules/vbn_create_stimulus_table/create_stim_table.py#L132-L225

I chose not to make ecephys_etl_pipelines a dependency of the SDK because

1. I believe we are going to need the flexibility to get_vsyncs from either the rising (in the case of things like running speed or rewards) or falling (in the case of stimulus presentations) edges of the sync file. The ecephys_etl code as implemented does not afford that flexibility

2. I wasn't sure how to guard against the ecephys_etl implementation changing out from under the SDK unexpectedly.

If it makes anyone feel better, ecephys_etl just copied sync_dataset.py from the SDK (which copied sync_dataset.py from some BitBucket repository somewhere).

In order to support this work, I had to break up our StimulusFile data objects into three classes.

BehaviorStimulusFile
ReplayStimulusFile
MappingStimulusFile

to represent the three varieties of stimulus file in the VBN experiments. The differences between the three come in how they are represented in our JSON serializations and how num_frames is accessed.

BehaviorStimulusFile corresponds to the StimulusFile class that was implemented for the VBO release.

The proper use of get_start_frames_from_stimulus_blocks is something like

from allensdk.brain_observatory.sync_stim_aligner import (
    _get_rising_times,
    get_start_frames_from_stimulus_blocks)

with SyncDataset(sync_path) as sync_data:
    frame_times = _get_rising_times(sync_data, sync_lines='vsync_stim')

start_frames = get_start_frames_from_stimulus_blocks(
                                    stimulus_files=[behavior_stim, mapping_stim, replay_stim],
                                    sync_file=sync_path,
                                    ...)

timesteps_for_behavior = frame_times[start_frames[0]: start_frames[0]+behavior_stim.num_frames()]
timesteps_for_replay = frame_times[start_frames[2]:start_frames[2]+replay_stim.num_frames()]

[morriscb]

Looks good overall. I think I've to one question and a comment that you should remove the explicit return None from a few functions.

[morriscb]

You don't need to explicitly return None as far as I know.

[morriscb]

Is it guaranteed that these two arrays are the same length?

[morriscb]

The three here looks to be also used on line 60. I realize this is in a fixture, but if you are use the same value in multiple places (and they need to be changed together if one changes) you should specify it as a variable.