-
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
Add time vector case to get_durations
.
#3118
Add time vector case to get_durations
.
#3118
Conversation
get_durations
.
We agree with this but we should check carfully that the internal use of this get_total_duration/get_dutaion is correcyly use anywhere in the code. |
Good point I will list their uses in the codebase below. I don't think any of these should be affected:
|
Oh yes @samuelgarcia the sorting analyzer needs changing too! How to do you think it is best to structure this, to avoid duplciating the generation code across sorter and recording? Can the |
@JoeZiminski since the analyzer can be recordingless, I would do the following:
Get total samples is always avalailable since it's saved as a recording attribute. |
Cool thanks @alejoe91 I will add this mode. For testing, I can't figure out how to make a recordingless analyser 😄 I guess it is not with |
Not the most elegant of solutions, but this works:
:) |
…add_time_vector_case_to_get_duration
@JoeZiminski I pushed a small fix in hybrid tools! Thanks for adding tests. Let us know when it's done! |
Thanks @alejoe91 will push final commit shortly! |
….com/JoeZiminski/spikeinterface into add_time_vector_case_to_get_duration
Hey @alejoe91 good to go from my end! |
This PR extends
BaseRecording.get_duration()
(and the associatedget_total_duration()
) but allowing it to handle thetime_vector
case. It is tested in #3117 (see that PR for why).In reality, this should only have an extremely minimal effect, only in cases with some irregularities in the time-interval spacing. Nonetheless I guess it makes sense to use the true
time_vector
for the duration if it exists as it will be different form estimating based on number of sample and sampling frequency.The calculation is
t_stop - t_start + sampling_step
. The sampling step is introduced due to fencepost error. For example, if you havets=0.1
,num_samples=10
,duration=1
, you will have10*0.1=1
, a 1s recording as expected. These will be represented by times[0, 0.1, ..., 0.9]
. Therefore we need to add 1xts
tot_stop - t_start
to get the true duration. As thets
is usually a very small number the duration calculated in this way is not nice and round as it usually is withnum_samples / sampling_frequency
.