Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Kilosort4 saves amp and Amplitude values by changing them. #632

Closed
hoyeon912 opened this issue Mar 26, 2024 · 2 comments
Closed

BUG: Kilosort4 saves amp and Amplitude values by changing them. #632

hoyeon912 opened this issue Mar 26, 2024 · 2 comments
Assignees
Labels

Comments

@hoyeon912
Copy link

Describe the issue:

Screenshot from 2024-03-26 18-33-41
This screenshot is captured from Kilosort4 files

Screenshot from 2024-03-26 18-33-53
and this one is from Kilosort2.5 files

Kilosort4 saves amp value in Amplitude and vice versa.
cluster_Amplitude file is also saved with amp value.
This bug makes several issues in phy2 which are waveform, cluster scatter, amplitude view.

Reproduce the bug:

Always occurs.

Error message:

No response

Version information:

I used Ubunut 20.04 and CUDA 11.8 in both kilosorts.
Kilosort 2.5 ran in MATLAB R2023b and Kilosort 4 ran in 3.9.19.

Context for the issue:

No response

Experiment information:

No response

@jacobpennington
Copy link
Collaborator

jacobpennington commented Mar 28, 2024

Hello, thanks for reporting this. Unfortunately it was not a simple case of variables being swapped, "amp" is actually generated by Phy and not something that we report directly. We've made some changes to the way templates are saved, and how cluster_Amplitudes are calculated, that should make those values comparable to previous versions. The changes are in #646 pending review, if you want to try it out with your data. Otherwise they'll probably be live in a few days unless we find other problems with it.

Found more problems after all, but we're working on it.

@jacobpennington
Copy link
Collaborator

Okay, this has been resolved with the changes pushed today, I'll add it to a new pip version soon.

For @hoyeon912 and anyone else that comes across this, please note that the values in the 'amp' and 'Amplitude' columns in Phy will likely not match previous versions, but we've updated them in the way that makes the most sense to us (and which fixes a related issue with templates and waveforms not having the same scale in Phy). The 'Amplitudes' column (computed by us) is the norm of the whitened templates. The 'amp' column (computed by Phy, we don't control this) is the largest max - min across channels for the un-whitened templates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants