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

WIP: Add tsDSS #30

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

WIP: Add tsDSS #30

wants to merge 3 commits into from

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented May 21, 2019

@drammock FYI I'm working on implementing tsDSS. My plan is:

  • Reformulate existing DSS code to use the same nomenclature from the AdC/Simon paper (@drammock feel free to look/confirm I did this properly so far)
  • Reformulate to prefer mne.compute_whitener for the whitening step, since it's careful about channel scalings, projectors, etc. (and just using _pca will not be)
  • Change the example to use the filtered data (faster)
  • Extend the DSS calculation to be time-shift capable (tsDSS)
  • Add simulation/array example from the paper

@coveralls
Copy link

coveralls commented May 21, 2019

Coverage Status

Coverage remained the same at ?% when pulling 82f915d on larsoner:tsdss into 6440770 on mne-tools:master.

Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

FYI, I am not very confident that my original implementation was sound --- I ended up rewriting this separately for the 2018 paper here. But it looks like you've re-implemented enough of it that I needn't worry.

mne_sandbox/preprocessing/_dss.py Show resolved Hide resolved
mne_sandbox/preprocessing/_dss.py Outdated Show resolved Hide resolved
mne_sandbox/preprocessing/_dss.py Outdated Show resolved Hide resolved
@drammock
Copy link
Member

@larsoner LMK if/when you want a more thorough review.

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

3 participants