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?

[zm711]

Yes. Exactly. I assume they are shifted (I didn't want to spend the time searching through the raw_data until someone else saw that the spike_times are wrong--ie I'm not sure how shifted and whether earlier or later than they should be). Thanks for confirming!

[alejoe91]

Yep! Here's an example from sortingview. The auto correlograms look correct, but waveforms and amplitudes (and so quality metrics etc.) are just noise. Should we comment on the Kilosort issue/repo?

[alejoe91]

Yep! Here's an example from sortingview. The auto correlograms look correct, but waveforms and amplitudes (and so quality metrics etc.) are just noise. Should we comment on the Kilosort issue/repo?

MouseLand/Kilosort#594 (comment)

[zm711]

Yeah we should. :)

[alejoe91]

@zm711 @JoeZiminski I deleted the containers with the fix because of the misalignment bug. Now we're back to the spike holes version for KS2.5 and KS3, which is the lesser evil :)

[zm711]

Agreed :)

[JoeZiminski]

Hey @zm711 @alejoe91 I had not tested the new version, good spot! Yes I agree that is the best approach, thanks!

[alirza67]

Hi there,
Is there any update on this?
I was wondering if the misalignment bug is fixed in the containers.

[alejoe91]

It's not fixed on Kilosort yet, unfortunately: MouseLand/Kilosort#623

Once a solution is found, we'll make sure to promptly build and push new containers.

[zm711]

Based on testing I think the holes are fixed. But did you see @alejoe91 that the holes were also in v2. So that would need a new container as well. Do we want to close once you have the new containers built?

[JoeZiminski]

Hey @alejoe91 @zm711 I think KS2-3 have stabilised, are the images fully up to date / can this issue be closed?