Skip to content

Fix KS4 for v>=4.0.5 and simplify skip correction#2774

Merged
alejoe91 merged 2 commits intoSpikeInterface:mainfrom
alejoe91:ks4-fix
Apr 29, 2024
Merged

Fix KS4 for v>=4.0.5 and simplify skip correction#2774
alejoe91 merged 2 commits intoSpikeInterface:mainfrom
alejoe91:ks4-fix

Conversation

@alejoe91
Copy link
Copy Markdown
Member

Fixes #2742

@alejoe91 alejoe91 added the sorters Related to sorters module label Apr 29, 2024
@alejoe91 alejoe91 requested a review from zm711 April 29, 2024 10:41
@alejoe91 alejoe91 changed the title Fix KS4 for v>=4.05 and simplify skip correction Fix KS4 for v>=4.0.5 and simplify skip correction Apr 29, 2024
Copy link
Copy Markdown
Member

@zm711 zm711 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I tested this locally both with and without drift correction ran fine on KS 4.0.6

Removal of parse maybe? But otherwise looks good to me.

from pathlib import Path
import os
from typing import Union
from packaging.version import parse
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are we using parse? I don't see it in the diff. Maybe the original idea was to treat different versions differently?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep indeed that was the original idea, removing :)

artifact_threshold=artifact,
file_object=file_object,
)
ops["nblocks"] = 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the KS based solution for skipping drift correction right?

I think it does make things simpler to just rely on their machinery. Maybe this will speed things up too from the wrapper?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, let's see :)

Comment thread src/spikeinterface/sorters/external/kilosort4.py Outdated
Co-authored-by: Zach McKenzie <92116279+zm711@users.noreply.github.com>
@alejoe91 alejoe91 merged commit 256757c into SpikeInterface:main Apr 29, 2024
@alejoe91 alejoe91 deleted the ks4-fix branch March 20, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sorters Related to sorters module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Kilosort4Sorter for v4.0.5

2 participants