elephant icon indicating copy to clipboard operation
elephant copied to clipboard

[Bug fix] Fix bug of synchrotool index out of bounds

Open morales-gregorio opened this issue 1 year ago • 12 comments

Hi,

The bug was a known issue, when applying the synchrotool if there was a synchrofact in the last bin the indices went out of bounds. Here a minimal code to reproduce the error:

import neo
import numpy as np
import quantities as pq
from elephant.spike_train_synchrony import Synchrotool

sampling_rate = 1/pq.ms
st = neo.SpikeTrain(np.arange(0, 11)*pq.ms, t_start=0*pq.ms, t_stop=10*pq.ms)

synchrotool_instance = Synchrotool([st, st], sampling_rate, spread=0)
synchrotool_instance.annotate_synchrofacts()

which (with the master branch elephant, python 3.9.7) produces the output:

---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
Cell In[2], line 5
      2 st = neo.SpikeTrain(np.arange(0, 11)*pq.ms, t_start=0*pq.ms, t_stop=10*pq.ms)
      4 synchrotool_instance = Synchrotool([st, st], sampling_rate, spread=0)
----> 5 synchrotool_instance.annotate_synchrofacts()

File elephant/elephant/spike_train_synchrony.py:414, in Synchrotool.annotate_synchrofacts(self)
    406 for idx, st in enumerate(self.input_spiketrains):
    407 
    408     # all indices of spikes that are within the half-open intervals
    409     # defined by the boundaries
    410     # note that every second entry in boundaries is an upper boundary
    411     spike_to_epoch_idx = np.searchsorted(
    412         right_edges,
    413         st.times.rescale(self.epoch.times.units).magnitude.flatten())
--> 414     complexity_per_spike = epoch_complexities[spike_to_epoch_idx]
    416     st.array_annotate(complexity=complexity_per_spike)
    418 self.annotated = True

IndexError: index 10 is out of bounds for axis 0 with size 10

The following is outdated To fix this issue I suggest a very simple change in the definition of the bins. To avoid spikes at bin edges (when binning at sampling resolution) the bins were shifted by half a bin width to the left. This leads to one additional (half)-bin at the end of the time series, which only raises the issue when a synchrofact is found in this last bin, which is only rarely in real data. Instead of shifting to the left, I propose shifting to the right, this leads to two initial spikes being in the first bin, but no extra bin at the end.

Let me know what you think, Aitor

PS: I see some of the tests fail with this change due to small differences, let me know if this change is ok and I will fix the tests

morales-gregorio avatar Dec 26 '23 19:12 morales-gregorio