KS2.5 / KS3 'spike holes' patches released

[JoeZiminski]

Hi Everyone,

A patch was recently released in KS2.5 and KS3 to fix the 'spike holes' bug. There were pushed as commits to the relevant branches in the KS repository.

For the next version of SI I makes sense to re-build the docker images from the KS main. However, if I understand correctly the versioning might be a bit tricky. Once the docker files are rebuilt, will all spikeinterface versions will begin pulling the most recent build? As I presume in some circumstances this change might lead to quite different KS outputs, so would be nice to pin to an SI version.

Also just to confirm, when building the images it is necessary to 1) download the Kilosort repository, then switch to the relevant KS version branch before building?

Cheers!

[samuelgarcia]

The KS docker do not contains spikeinterface inside.
spikeinterface is installating spikeinetrface in the container on the fly.
We now have a mechanism to taget a specific spikeinterface version inside the conatiner that can be different from the host. So it should help. No ?

About this bug, this is absolutly amazing that no one discover this enormous conceptual bug before 2024.
7ms missing for evry 2000ms is not small for me.
Almost every lab is using KS2.5 or KS3 and a simple spike rate over time with a small bin should have enhance this!
no ?

About when to constuct theses new docker image, I don't known, it is a bit tedious.
@alejoe91 will you have time ?

[alejoe91]

Yes, I'll try to do it by the end of the week

[JoeZiminski]

spikeinterface is installating spikeinetrface in the container on the fly.
We now have a mechanism to taget a specific spikeinterface version inside the conatiner that can be different from the host. So
it should help. No ?

That's perfect yes I thought all SI versions might pull the most recently compiled image, awesome!

Yes I agree it's amazing it's not been picked up after all this time, I guess it required the right kind of visualisation to make the problem obvious.

Thanks @alejoe91 that's great, let me know if I can help.

Thanks both!

[JoeZiminski]

Actually @alejoe91 @samuelgarcia, on the issue the discussion is ongoing with Olivier and Gaelle yet to test, maybe it makes sense to wait before building to ensure those patches have solved the problem.

[alejoe91]

I'll start a local build to make sure that everything works smoothly :)

We can wait a few days to upload to dockerhub

[zm711]

Small thing, but Kilosort now has this warning on their read me

Warning ‼️: There was a bug in Kilosort 2.5 and 3 (but not 1,2 and 4) which caused fewer spikes to be detected in ~7ms periods at batch boundaries (every 2.1866s, issue #594). The patch0 releases fix this bug. It is also advised not to manually change the batch size in any Matlab-version of Kilosort (1-3).

Should this be added to spikeinterface params readme for kilosorts 1-3?

[alejoe91]

@JoeZiminski I'm actually going to go ahead and push the containers with the fixes from Marius so we can fix that the issue is solved on our side too.

[alejoe91]

Done:

• https://hub.docker.com/repository/docker/spikeinterface/kilosort2_5-compiled-base/general
• https://hub.docker.com/repository/docker/spikeinterface/kilosort3-compiled-base/general

[zm711]

@JoeZiminski Have you tested this. My lab tested this and we are seeing some weird offsets that make waveform extraction (in Phy at least) wrong..... I'm a little worried that maybe patching this bug introduced something else, so I would love to know if someone else has tested the patch and gotten it to work nicely on data.

[alejoe91]

@zm711 I think I'm observing the same! Basically the spike trains seem to be shifted and we also get waveforms that are completely noisy! Is that what you're seeing too?