-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inconsistent handling of time offsets in plot_traces #3324
Comments
Hey @jonahpearl thanks for this, so to be sure I understand correctly:
I think this is because the Now, this conditional is outdated as
@alejoe91 @samuelgarcia do you agree? In practice, can we deprecate |
Hi @JoeZiminski — yes, running One other note, if I run that fix, and then "naively" ask for a time segment outside the start of the recording with the plot_traces function, I get a fairly cryptic ValueError because it's trying to run operations on an empty array, it might be worth checking for this and raising a user-readable error, since I imagine some people will just instinctively try to plot the first few seconds of their recording. ValueError TracebackValueError Traceback (most recent call last)
Cell In[19], line 4
1 fig, ax = plt.subplots(figsize=(15,20))
3 iids = slice(39,40)
----> 4 si.plot_traces(
5 recording_car,
6 backend='matplotlib',
7 order_channel_by_depth=True,
8 # time_range=(25.5, 26.5), # started by 34
9 time_range=(1,2),
10 channel_ids=recording_car.channel_ids[iids],
11 mode="line",
12 ax=ax,
13 show_channel_ids=True,
14 )
15 _ = plt.yticks(np.arange(iids.stop - iids.start), recording_car.channel_ids[iids])
File ~/datta-lab/spikeinterface/src/spikeinterface/widgets/traces.py:170, in TracesWidget.__init__(self, recording, segment_index, channel_ids, order_channel_by_depth, time_range, mode, return_scaled, cmap, show_channel_ids, events, events_color, events_alpha, color_groups, color, clim, tile_size, seconds_per_row, scale, with_colorbar, add_legend, backend, **backend_kwargs)
168 traces0 = list_traces[0]
169 mean_channel_std = np.mean(np.std(traces0, axis=0))
--> 170 max_channel_amp = np.max(np.max(np.abs(traces0), axis=0))
171 vspacing = max_channel_amp * 1.5
173 if rec0.get_channel_groups() is None:
File ~/miniconda3/envs/spikeinterface/lib/python3.9/site-packages/numpy/core/fromnumeric.py:2810, in max(a, axis, out, keepdims, initial, where)
2692 @array_function_dispatch(_max_dispatcher)
2693 @set_module('numpy')
2694 def max(a, axis=None, out=None, keepdims=np._NoValue, initial=np._NoValue,
2695 where=np._NoValue):
2696 """
2697 Return the maximum of an array or maximum along an axis.
2698
(...)
2808 5
2809 """
-> 2810 return _wrapreduction(a, np.maximum, 'max', axis, None, out,
2811 keepdims=keepdims, initial=initial, where=where)
File ~/miniconda3/envs/spikeinterface/lib/python3.9/site-packages/numpy/core/fromnumeric.py:88, in _wrapreduction(obj, ufunc, method, axis, dtype, out, **kwargs)
85 else:
86 return reduction(axis=axis, out=out, **passkwargs)
---> 88 return ufunc.reduce(obj, axis, dtype, out, **passkwargs)
ValueError: zero-size array to reduction operation maximum which has no identity AssertionError TracebackAssertionError Traceback (most recent call last)
Cell In[20], line 1
----> 1 tmp_rec = recording_car.time_slice(1, 2)
File ~/datta-lab/spikeinterface/src/spikeinterface/core/baserecording.py:727, in BaseRecording.time_slice(self, start_time, end_time)
724 start_frame = self.time_to_sample_index(start_time) if start_time else None
725 end_frame = self.time_to_sample_index(end_time) if end_time else None
--> 727 return self.frame_slice(start_frame=start_frame, end_frame=end_frame)
File ~/datta-lab/spikeinterface/src/spikeinterface/core/baserecording.py:702, in BaseRecording.frame_slice(self, start_frame, end_frame)
684 """
685 Returns a new recording with sliced frames. Note that this operation is not in place.
686
(...)
697 A new recording object with only samples between start_frame and end_frame
698 """
700 from .frameslicerecording import FrameSliceRecording
--> 702 sub_recording = FrameSliceRecording(self, start_frame=start_frame, end_frame=end_frame)
703 return sub_recording
File ~/datta-lab/spikeinterface/src/spikeinterface/core/frameslicerecording.py:37, in FrameSliceRecording.__init__(self, parent_recording, start_frame, end_frame)
35 start_frame = 0
36 else:
---> 37 assert 0 <= start_frame < parent_size
39 if end_frame is None:
40 end_frame = parent_size
AssertionError: |
I think this should be fixed if we use |
@jonahpearl could you test this? #3393 |
Also improved error messages in that PR |
Actually doesn't seem to work for me — when I subselect my one channel, I just get an empty plot, even without passing any time_range. If I don't subselect the channels it works. And you can see that channel has data b/c if i pull it out directly it's fine. |
Hi all — I was doing some manual digging around to deal with artifacts caused by some custom lighting in our rigs, and I wanted to find the moment in the recording where the artifacts began. I noticed that depending on whether I used
recording.time_slice()
orrecording.plot_traces
, there are two different behaviors for dealing with the case where t0 != 0 (this is coming from open ephys, so it's often the case that the first timestamp isn't 0 in the recording).Case 1: the "give me X many seconds into the recording" strategy. This appears to be used by
recording.plot_traces
, and I think is a bug.Case 2: the "give me the data where the timestamps equal this value" strategy. This appears to be used by
recording.time_slice
, and I think is the correct way (?).For example, here is the plot from
plot_traces
for the time range (25.5, 26.5): you can see the artifacts start ~halfway through the window.Now if I run this code to try to show the same thing but with
recording.time_slice()
, we instead get a different moment:If we adjust the time slicing to account for the inconsistent behavior, we recover the same moment in the data:
You can see the behavior in the code. time_slice ultimately relies on this logic (at least in my case, it seems that self.t_start is assigned automatically when reading from OE folders):
sample_index = (time_s - self.t_start) * self.sampling_frequency
, which means that, say, if t0 were 10, and the user requested t=20, the code would correctly give the user the data from 10 seconds into the recording.However the plot/get_traces functions don't appear to do this correction, as seen here and down on L145 below that, and then the helper function just inherits that time range directly.
Hopefully fixing the behavior in
plot_traces
is easy enough and won't hurt anyone's workflow — I imagine that may be why the bug still exists :)The text was updated successfully, but these errors were encountered: