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

Hard-coded spacing constraints crashing Kilosort 3.0 #313

Closed
czuba opened this issue Feb 1, 2021 · 4 comments · Fixed by #595
Closed

Hard-coded spacing constraints crashing Kilosort 3.0 #313

czuba opened this issue Feb 1, 2021 · 4 comments · Fixed by #595

Comments

@czuba
Copy link
Contributor

czuba commented Feb 1, 2021

Hard-coded spacing constraints crashing Kilosort 3.0

... seems to be the issue, but please confirm & consider

Running Kilosort 3.0 on 32ch U-probes (stereotrode config: 50um within [x], 100um between [y]; 1,500um total length)
Crashing during find_merges.m

% sort by firing rate first
nspk = accumarray(rez.st3(:,2), 1, [Nk, 1], @sum);

rez.st3(:,2) contains many zeros that originate during final_clustering.m

  • On line 132, initial st3(:,2) is replaced by hid, which is derivative of run_pursuit.m outputs
  • I suspect the hiccup actually happens earlier, on line 68, when encountering a hard-coded (electrode spacing?) constraint of < 20

Presumably this should/could be something like dmin instead...

@marius10p
Copy link
Contributor

That was supposed to be dmin indeed., I fixed it now. I also added a horizontal pitch detection for spike templates in the spike extractor, so that in your case it should use three different X positions: 0, 25 and 50.

@marius10p
Copy link
Contributor

Please let me know if the latest commits have fixed this.

@czuba
Copy link
Contributor Author

czuba commented Feb 3, 2021

Not quite working. Currently referencing rez.ops.dmin in standalone_detector.m before having been created in extract_spikes.m.

...replacing with simple line of code from extract_spikes.m:

dNearActiveSite = median(diff(unique(rez.yc)));

did the trick, but wouldn't we actually want something that also takes x distance into consideration? ...as I'd expect getClosestChannels2() does in standalone_detector.m, line 26?


Not sure I follow the "three different X positions" statement from your previous comment.
I currently use x coordinates +/-25:

xc = repmat( [-25;25], nChanTOT/2, 1);

...is that not correct/compatible?

@marius10p
Copy link
Contributor

Thanks, fixed it now. That condition will only kick in for non-grid-like geometries, so it should be fine in most cases to set it to a reasonable number like the vertical pitch.

"ycup, xcup" are the centers of the template prototypes used to extract spikes both for drift correction and for the first clustering. This spike detector is described in the Neuropixels 2 paper. It's good to have some that are centered in-between sites, but the defaults were only good for Neuropixels. Now they should be ok for you as well, with templates centered at half-integer site spacings horizontally and vertically. Also, I forgot to make them consistent between datashift2 and extract_spikes, and I updated that just now.

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

Successfully merging a pull request may close this issue.

2 participants