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

KS3 fixes: do-over pull request #324

Closed
wants to merge 4 commits into from
Closed

KS3 fixes: do-over pull request #324

wants to merge 4 commits into from

Conversation

czuba
Copy link
Contributor

@czuba czuba commented Feb 3, 2021

Core

  • parameterization fixes to extract_spikes.m & final_clustering.m
  • GUI: don't crash if default WindowStyle == 'docked'
  • split out faster version of get_whitening_matrix.m

Cosmetic

  • GUI: parameterize font size
  • Don't reuse figure for datashift plot

- impose limit to NchanNear for recordings with <100 channels
- GUI: parameterize font size
- GUI: don't crash if default WindowStyle == 'docked'
- Don't reuse figure for datashift plot
- Mmuch faster loading of data for computing whitening matrix
- - Activate by setting ops.useMemMapping = true
- - same method would be great when loading & saving filtered dat,
    but cannot parse method of including overlapping buffers w/in
    filtered data output (can buffer sampling not be done on GPU??)

** svdecon.m was inconsistently performing gpuArray gather,
leading to error attempting to convert "to single from gpuArray"
(Matlab 2019a, Ubuntu 18.04). Added a gather where looked least
intrusive, but just a guess...
…ng data loading in get_whitening_matrix_faster.m
- reverted original copy of get_whitening_matrix.m
  - replace with `get_whitening_matrix_faster.m` on linux to speed up loading with memmapping & parfor
@marius10p
Copy link
Contributor

Thanks. I see that you pushed your commits over the previous commits, undoing some and changing some. Sorry, but can you please start with a clean repo and add the minimal changes you want to make to that? Currently I cannot verify if the things you added and then taken back have reverted those functions to their original.

Also, we don't need a zero-crossing value (that's what ops.nt0min is), and I think sig doesn't need to be parametrized either: it's a property of the waveforms in tissue, i.e. the minimum spatial distance over which we expect waveforms to change.

@czuba
Copy link
Contributor Author

czuba commented Feb 4, 2021

Submitted another PR with these changes merged with upstream changes that you'd already made into other commits.

  • Yep, I'd already dug up the ops.nt0min parameter & updated to use it...looks like it retroactively added to this PR. Should be in the new PR as well.
  • ops.sig was already parameterized elsewhere
    • good to know has physiological origins; and good to have consistency throughout codebase should a user choose/need to adjust parameters to achieve usable results

@czuba czuba closed this Feb 4, 2021
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 this pull request may close these issues.

None yet

2 participants