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

Template padding for phy and plotting in GUI #66

Closed
djoshea opened this issue May 16, 2019 · 2 comments · Fixed by #595
Closed

Template padding for phy and plotting in GUI #66

djoshea opened this issue May 16, 2019 · 2 comments · Fixed by #595
Assignees

Comments

@djoshea
Copy link
Contributor

djoshea commented May 16, 2019

The waveform templates are nt0 samples long (which defaults to 61), with the trough of the waveform occurring at ops.nt0min which defaults to 20. When exporting to Phy, the Kilosort2 code pre-pads the templates with nt0min+1 zeros at splitAllClusters.m L192 at

rez.Wphy = cat(1, zeros(1+ops.nt0min, Nfilt, Nrank), rez.W);

where rezToPhy uses rez.Wphy to construct the templates. So this gets you 1 + 20 + 61 = 82 samples, which puts the trough at 1 + 20 + 20 = 41 samples into the waveform, which is centered within the template. This is where Phy expects to line up the template against the list of spike times.

I suspect the problem is that while this works for the default values, the correct amount of padding to achieve centering would be nt0 - 2*nt0min - 1, i.e.

[ nt0 - 2*nt0min - 1 ] [ nt0min ]  |  [ nt0 - nt0min ] 
     zero padding      pre-trough  |    post-trough
             [ nt0 - nt0min - 1 ]  |  [ nt0 - nt0min ] 

If this is correct, maybe changing splitAllClusters.m L192 to this instead would ensure the samples stay centered?

rez.Wphy = cat(1, zeros(ops.nt0 - 2*ops.nt0min - 1, Nfilt, Nrank), rez.W);

I haven't tested this yet with a different nt0min value, will try and see what happens.

Changing ops.nt0min or ops.nt0 also would affect plotting in the GUI, as currently the offset of -19 is hardcoded in predictData.m L34

theseSamps = st(s)+(1:buff)-19-samps(1)+buff*2;

This might also be tangentially related to #63, although there the templates don't just look offset. I wonder if there's a conceptually similar issue where the templates are fit to data using the original hardcoded nt0min of 20 and the waveforms are extracted using the actual nt0min value? Seems unlikely.

@marius10p
Copy link
Contributor

Good catch, I think you are right. Happy to change the padding, though I am not sure if Phy expects an odd or even number of samples @rossant?

@nsteinme can you please make the change in the GUI code? I think 19 has to be replaced by nt0min-1.

@marius10p marius10p self-assigned this Jun 11, 2019
@rossant
Copy link
Contributor

rossant commented Jun 11, 2019

phy uses traces[s-k:s+ns-k] in the trace view, where k = ns//2, s is the spike time, and ns is the number of samples (odd or even)

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.

3 participants