AllenInstitute / AllenSDK

code for reading and processing Allen Institute for Brain Science data
https://allensdk.readthedocs.io/en/latest/
Other
343 stars 149 forks source link

Incorrect presentation and use of "process_" functions leads to incorrect latency values. #37

Open davestockton opened 7 years ago

davestockton commented 7 years ago

The example seen at http://alleninstitute.github.io/AllenSDK/_static/examples/nb/cell_types.html#Computing-Electrophysiology-Features shows the user setting stim_start = 1.0 and passing it to the fx.process_instance("", v, i, t, stim_start, stim_duration, "") function as a parameter. In the nwb files, however, the SS and LS stimuli start at index 204000, which is actually 0.02 seconds after 1.0. The SDK software actually calculates latency not from the start of the stimulus, but from the start of the analysis window, which therefore makes calculations on the webpage, and in fact the parameter list, incorrect. This error can cause the feature extraction software to miss spikes associated with Short Squares if the analysis window is short, since 0.02 seconds is larger than the duration of the Short Square stimulus (0.003 seconds).

It may make sense that the parameters be changed to "analysis_start" and "analysis_duration" (which appears to be the actual use of the two parameters), an additional parameter be added to the parameter list ("stimulus_start"), and that the calculations having to do with timing in reference to stimulus start be corrected.

gouwens commented 7 years ago

Thanks for catching that. We have rewritten feature analysis code, so we should update that example to use the new version. It is true that the parameter labels are inexact as you say. In the new version, there are just "start" and "end" parameters (http://alleninstitute.github.io/AllenSDK/_modules/allensdk/ephys/ephys_extractor.html#EphysSweepFeatureExtractor).

The extractor (both the old and new versions) will give you spike times relative to the start of the entire sweep, so you can still calculate a latency to whatever point you want, regardless of what analysis window you are using; you don't need to rely on the latency value calculated by the extractor. So it may make more sense to leave that to the user to do on his or her own rather than including an extra "stimulus start" parameter in the extractor.

davestockton commented 7 years ago

I see... perhaps it would be good to relabel that particular latency as "analysis window latency" because it appears on the surface (without extensive code inspection) to be the latency of the first spike post-stimulus start. At this point it isn't possible to use the SDK properly without spending a huge amount of time examining the code and the nwb file (at least that was my experience).