spikeinterface icon indicating copy to clipboard operation
spikeinterface copied to clipboard

Implemented RT-Sort as an external sorter

Open max-c-lim opened this issue 11 months ago • 10 comments

max-c-lim avatar Jan 06 '25 04:01 max-c-lim

Hello, this looks great! A few things:

  • It'd be great to add some details to the docs. You can add the sorter to the list of supported sorters in doc/modules/sorters.rst and add the reference to your paper in doc/references.rst.
  • Could you add the basic sorter test suite please? This does some very basic testing for sorters. It should be straightforward. For an example, see src/spikeinterface/sorters/external/tests/test_herdingspikes.py
  • I've had an issue running the sorter on my data (https://github.com/braingeneers/braindance/issues/2). This isn't really related to the implementation of the sorter in spikeinterface, but I'll be able to test the PR a bit better if I can run it on a dataset I know well :)

Thanks!

chrishalcrow avatar Jan 06 '25 08:01 chrishalcrow

Hi. This is a super good news to have a new sorter here. Welcome to RT-sort. Welcome in the game. Thanks for the work. I will have look also.

samuelgarcia avatar Jan 06 '25 16:01 samuelgarcia

Thank you for your fast review!

In the first commit, RT-Sort was added to the "Supported Spike Sorters" section. Is there another section where I should add it?

In the most recent commit, I added a reference to my paper in doc/references.rst and created src/spikeinterface/sorters/external/tests/test_rtsort.py.

I'll resolve the error with your data next. Thanks for documenting the issue so well and providing example data.

max-c-lim avatar Jan 07 '25 01:01 max-c-lim

@max-c-lim

I think it would be great if you published RT-sort as a python package with all its dependencies. That would make installation instructions and the is_installed method much easier

alejoe91 avatar Jan 07 '25 11:01 alejoe91

Thank you for all your comments! I simplified the installation process and implemented braindance.__version__.

max-c-lim avatar Jan 20 '25 00:01 max-c-lim

My branch (before merging main) failed the tests Complete tests / windows-latest Python 3.9, Complete tests / windows-latest Python 3.12, and Complete tests / windows-latest Python 3.12.

I may be mistaken, but I believe that all three failed because of the same reason: the command git clone --filter=blob:none --quiet https://github.com/SpikeInterface/probeinterface.git raised the error invalid path 'resources/wiring_references/ASSY-116>RHD2132.pdf'. My branch does not use or change probeinterface or RHD2132.pdf.

Could someone please advise me on how I could resolve this error?

Thanks!

max-c-lim avatar Feb 12 '25 00:02 max-c-lim

@max-c-lim that has been fixed! Can you try again?

@samuelgarcia tests fail for tdc1. Do you plan to update it to support numpy 2.0?

alejoe91 avatar Feb 12 '25 09:02 alejoe91

@max-c-lim I finally got to test this, but I'm getting a very weird behavior...

Here's a simple test script:

import spikeinterface.full as si

rec, sort = si.generate_ground_truth_recording(
    num_channels=32,
    num_units=10,
    durations=[60],
    seed=100
)

# basic preprocessing
rec_pre = si.bandpass_filter(rec)
rec_pre = si.common_reference(rec_pre)

# run RT-sort
sort_rt = si.run_sorter("rt-sort", rec, remove_existing_folder=True, verbose=True)

Which seems to run fine:

Saving traces:
Alllocating disk space for traces ...
Extracting traces
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 32[/32](http://localhost:8888/32) [00:08<00:00,  3.70it[/s](http://localhost:8888/s)]
Running detection model:
Compiling detection model for 32 elecs ...
Allocating disk space to save model traces and outputs ...
Inference scaling: 2.234049938020188
Running model ...
100%|███████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 8332[/8332](http://localhost:8888/8332) [00:04<00:00, 2075.80it[/s](http://localhost:8888/s)]
Detecting sequences
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 32[/32](http://localhost:8888/32) [00:06<00:00,  4.85it[/s](http://localhost:8888/s)]
Detected 62 preliminary propagation sequences
Extracting sequences' detections, intervals, and amplitudes
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 62[/62](http://localhost:8888/62) [00:01<00:00, 58.16it[/s](http://localhost:8888/s)]
59 clusters remain after filtering
Reassigning spikes to preliminary propagation sequences
Initializing ...
Sorting recording
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 15000[/15000](http://localhost:8888/15000) [00:12<00:00, 1230.43it[/s](http://localhost:8888/s)]
Extracting sequences' detections, intervals, and amplitudes

100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 55[/55](http://localhost:8888/55) [00:01<00:00, 47.09it[/s](http://localhost:8888/s)]
55 clusters remain after filtering
Merging preliminary propagation sequences - first round
100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 12[/12](http://localhost:8888/12) [00:00<00:00, 114.52it[/s](http://localhost:8888/s)]
16 sequences after first merging
Merging preliminary propagation sequences - second round ...

RT-Sort detected 8 sequences
Sorting recording
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 15000[/15000](http://localhost:8888/15000) [00:16<00:00, 911.13it[/s](http://localhost:8888/s)]
rt-sort run time 63.83s

Here comes the issue. The recording has 1500000 frames (since sampling freq is 25'000), but RT-sort returns sample indices as large as 37487200000!!

See this:

print(rec.get_num_samples())
>>> 1500000

print(max(sort_rt.to_spike_vector()["sample_index"]))
>>> 37487200000

Clearly, this causes all sort of issues downstream... @max-c-lim can you check what's going on?

alejoe91 avatar Mar 10 '25 10:03 alejoe91

@max-c-lim I finally got to test this, but I'm getting a very weird behavior...

Here's a simple test script:

import spikeinterface.full as si

rec, sort = si.generate_ground_truth_recording(
    num_channels=32,
    num_units=10,
    durations=[60],
    seed=100
)

# basic preprocessing
rec_pre = si.bandpass_filter(rec)
rec_pre = si.common_reference(rec_pre)

# run RT-sort
sort_rt = si.run_sorter("rt-sort", rec, remove_existing_folder=True, verbose=True)

Which seems to run fine:

Saving traces:
Alllocating disk space for traces ...
Extracting traces
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 32[/32](http://localhost:8888/32) [00:08<00:00,  3.70it[/s](http://localhost:8888/s)]
Running detection model:
Compiling detection model for 32 elecs ...
Allocating disk space to save model traces and outputs ...
Inference scaling: 2.234049938020188
Running model ...
100%|███████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 8332[/8332](http://localhost:8888/8332) [00:04<00:00, 2075.80it[/s](http://localhost:8888/s)]
Detecting sequences
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 32[/32](http://localhost:8888/32) [00:06<00:00,  4.85it[/s](http://localhost:8888/s)]
Detected 62 preliminary propagation sequences
Extracting sequences' detections, intervals, and amplitudes
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 62[/62](http://localhost:8888/62) [00:01<00:00, 58.16it[/s](http://localhost:8888/s)]
59 clusters remain after filtering
Reassigning spikes to preliminary propagation sequences
Initializing ...
Sorting recording
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 15000[/15000](http://localhost:8888/15000) [00:12<00:00, 1230.43it[/s](http://localhost:8888/s)]
Extracting sequences' detections, intervals, and amplitudes

100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 55[/55](http://localhost:8888/55) [00:01<00:00, 47.09it[/s](http://localhost:8888/s)]
55 clusters remain after filtering
Merging preliminary propagation sequences - first round
100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 12[/12](http://localhost:8888/12) [00:00<00:00, 114.52it[/s](http://localhost:8888/s)]
16 sequences after first merging
Merging preliminary propagation sequences - second round ...

RT-Sort detected 8 sequences
Sorting recording
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 15000[/15000](http://localhost:8888/15000) [00:16<00:00, 911.13it[/s](http://localhost:8888/s)]
rt-sort run time 63.83s

Here comes the issue. The recording has 1500000 frames (since sampling freq is 25'000), but RT-sort returns sample indices as large as 37487200000!!

See this:

print(rec.get_num_samples())
>>> 1500000

print(max(sort_rt.to_spike_vector()["sample_index"]))
>>> 37487200000

Clearly, this causes all sort of issues downstream... @max-c-lim can you check what's going on?

@alejoe91 Thank you for testing RT-Sort! I'm sorry for the delay in resolving this: the issue was with the code for RT-Sort and not with this fork.

Could you please try again after reinstalling RT-Sort with pip install --force-reinstall git+https://github.com/braingeneers/braindance#egg=braindance[rt-sort]? On my end, print(max(sort_rt.to_spike_vector()["sample_index"])) results in 1499488, but this number might be a little different on your end.

Thanks!

max-c-lim avatar Mar 24 '25 02:03 max-c-lim

Hi @max-c-lim

Got the same weird behavior with the latest commit after installing from source!

alejoe91 avatar Apr 25 '25 17:04 alejoe91

Let's postpone until this issue on RT-sort is fixed: https://github.com/braingeneers/braindance/issues/4

alejoe91 avatar Jul 17 '25 17:07 alejoe91