INM-6 / swan

Swan (Sequential Waveform Analyzer) is an open-source graphical tool for tracking single units across multiple sessions of electrophysiological data that was recorded using chronically implanted microelectrode arrays.
BSD 3-Clause "New" or "Revised" License
5 stars 2 forks source link

Update dependencies to Python 3.8, neo==0.9.0, elephant==0.9.0 #9

Open mdenker opened 3 years ago

mdenker commented 3 years ago

Swan should be based on the latest releases of the neo and elephant tools. Most problematic would be neo, as it has undergone a number of breaking changes.

shashwatsridhar commented 3 years ago

@mdenker

I was working on getting the data loading in SWAN to work with neo 0.9.0, but the new loading routines in neo are obviously different from the old ones. So I was wondering:

1) Should we continue to use the blackrockio_v4.BlackrockIO or should we try and migrate to blackrockio.BlackrockIO? (I'm assuming we should go with the latter)

2) If we go with the latter, I don't see a clear way to load just one channel at a time using BlackrockIO. The read_block() or the read() methods don't have a way of specifying which channels to load. One way around this would be to lazy load the data and somehow query the required channel each time. However, I haven't been able to figure out how this would work in neo 0.9.0, since there are no ChannelIndex objects contained in the block/segment, nor the equivalent Group objects. I only get a neo Block with one Segment and multiple SpikeTrain objects in that segment (I'm trying this with the file i140703-001-03.nev from NIkos2). Do you have any ideas how we could do this?

JuliaSprenger commented 3 years ago

@mdenker

I was working on getting the data loading in SWAN to work with neo 0.9.0, but the new loading routines in neo are obviously different from the old ones. So I was wondering:

1. Should we continue to use the `blackrockio_v4.BlackrockIO` or should we try and migrate to `blackrockio.BlackrockIO`? (I'm assuming we should go with the latter)

Yes, the blackrockio_v4 is deprecated so it would be wiser to switch now than later.

2. If we go with the latter, I don't see a clear way to load just one channel at a time using `BlackrockIO`. The `read_block()` or the `read()` methods don't have a way of specifying which channels to load. One way around this would be to lazy load the data and somehow query the required channel each time. However, I haven't been able to figure out how this would work in neo 0.9.0, since there are no `ChannelIndex` objects contained in the block/segment, nor the equivalent `Group` objects. I only get a neo `Block` with one `Segment` and multiple `SpikeTrain` objects in that segment (I'm trying this with the file `i140703-001-03.nev` from NIkos2). Do you have any ideas how we could do this?

With the new IO concept you can either load the complete dataset (lazy=False), or you lazy load the data and then select a particular lazy proxy object (ProxySpiketrain) and create a regular spiketrain via it's .load() method.

You are right that ChannelIndex objects were removed in favour of Group and ChannelView objects. Here you can use the new parameter create_group_across_segment of read_block to create something similar to the ChannelIndex you were using. Here is an excerpt of the docstring:

        :param create_group_across_segment: bool or dict
            If True :
              * Create a neo.Group to group AnalogSignal segments
              * Create a neo.Group to group SpikeTrain across segments
              * Create a neo.Group to group Event across segments
              * Create a neo.Group to group Epoch across segments
            With a dict the behavior can be controlled more finely
            create_group_across_segment = { 'AnalogSignal': True, 'SpikeTrain': False, ...}
mdenker commented 3 years ago

Thanks for the heads-up, @JuliaSprenger.

Indeed, the proxyspike train lazy-loading may well be super helpful to speed up SWAN -- maybe even making the not necessary anymore.

JuliaSprenger commented 3 years ago

Depending how much you want to invest into the performance optimization potentially also just using the rawio layer of Neo might be interesting to you. Then you can directly access chunks of the data files, without having the nice SpikeTrain Quantity wrapping around it. But I guess as SWAN is only interested in spikes and not continuous signals this might be a bit too much effort for this relatively small amount of data.

mdenker commented 3 years ago

Yes, in principle, most of swan needs the waveforms only except for the rate histograms... It's a good point though -- maybe for other file formats in the future, such as nix, one may want to use the raw signals in case no .waveforms exists -- in that case maybe using the rawio layer becomes imperative for speed...

shashwatsridhar commented 3 years ago

Thanks @JuliaSprenger for the hint. I tried this, and I run into an error with the i140703-001.nev (Nikos2) file when I set create_group_across_segments to True. The error is:

filename = "i140703-001-03.nev"
blio = BlackrockIO(filename)
block = blio.read_block(lazy=True, create_group_across_segment=True, load_waveforms=True)
---------------------------------------------------------------------------
UnboundLocalError                         Traceback (most recent call last)
<ipython-input-9-2e9aa8c5cf42> in <module>
----> 1 blio.read_block(lazy=True, create_group_across_segment=True, load_waveforms=True)

~/miniconda3/envs/swan/lib/python3.9/site-packages/neo/io/basefromrawio.py in read_block(self, block_index, lazy, create_group_across_segment, signal_group_mode, load_waveforms)
    154             st_groups = []
    155             for c in range(unit_channels.size):
--> 156                 group = Group(name='SpikeTrain group {}'.format(i))
    157                 group.annotate(unit_name=unit_channels[c]['name'])
    158                 group.annotate(unit_id=unit_channels[c]['id'])

UnboundLocalError: local variable 'i' referenced before assignment

Does the data file need to be updated to support the new neo version, somehow? Or do you think this is a bug I should report?

shashwatsridhar commented 3 years ago

On a closer look, looks like a bug - the variable i should in fact be c, and this hasn't been fixed on master either. Should I file a bug report?

JuliaSprenger commented 3 years ago

Yes, indeed, this is a bug. Feel free to either open an issue or submit a PR (ideally including a test with the parameters you used to discover this bug).

shashwatsridhar commented 3 years ago

I have fixed the issue in my fork, but I am really not sure how I'd write a meaningful test for this. The errors in the code were syntactical, not logical. Does that still warrant a test?

I'll make an issue and a corresponding PR. If you have ideas on how I can write a good test, we could discuss it there! :)