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

cannot run MDM example #40

Closed
nbara opened this issue Aug 23, 2016 · 13 comments · Fixed by #41
Closed

cannot run MDM example #40

nbara opened this issue Aug 23, 2016 · 13 comments · Fixed by #41

Comments

@nbara
Copy link

nbara commented Aug 23, 2016

Hi Alexandre,

I was trying the cross-validation classification example code from the homepage on some toy EEG data (64 channels), but I keep running into the following error:

screen shot 2016-08-23 at 15 33 31

I tracked the error down to the mean_riemann method . logm(tmp) (line 61) is filled with NaNs.

My input data is in the following format (Ntrials, Nchannels, Nsamples) :

screen shot 2016-08-23 at 15 38 48

Let me know if you need more information.

Thanks!

@alexandrebarachant
Copy link
Collaborator

alexandrebarachant commented Aug 23, 2016

This is a common error when the data are rank deficient (high dimension or when a common average reference has been applied).
you have to regularize the covariance matrices by using Covariances(estimator='lwf')

I will make a FAQ for this one.

you can also check out the example section of the doc for more examples

@nbara
Copy link
Author

nbara commented Aug 23, 2016

Aah, I should have thought of that ! Indeed I've removed eyeblinks w/ ICA !

Thanks for the prompt response :)

@kingjr
Copy link
Contributor

kingjr commented Aug 23, 2016

Note that decoding is generally resistant to eye artefacts, so it might
actually not be a great idea to do an ICA-based clean up.

Le 23 août 2016 16:03, "nbara" notifications@github.com a écrit :

Aah, I should have thought of that ! Indeed I've removed eyeblinks w/ ICA !

Thanks for the prompt response :)


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#40 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEp7DJOHj439k8GRug4P1pOqQCER4Sjrks5qiv28gaJpZM4Jq72m
.

@nbara
Copy link
Author

nbara commented Aug 23, 2016

Noted, thanks!

@kingjr
Copy link
Contributor

kingjr commented Aug 23, 2016

Alex, I think an explicit test/ error could help, wdyt?

Le 23 août 2016 16:15, "nbara" notifications@github.com a écrit :

Noted, thanks!


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#40 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEp7DBDbdNiyUpOPu2RlG4eF0Vd5p74Zks5qiwCFgaJpZM4Jq72m
.

@alexandrebarachant
Copy link
Collaborator

yep, i was thinking about the same thing.
we could catch the error and trow something more explicit.

i'm not found of testing for positivness, as the check in itself will slow everything down.

@kingjr
Copy link
Contributor

kingjr commented Aug 23, 2016

Right, catching the nan should suffice. Is there any other reasons to get
nan than non positiveness?

Le 23 août 2016 16:33, "alexandre barachant" notifications@github.com a
écrit :

yep, i was thinking about the same thing.
we could catch the error and trow something more explicit.

i'm not found of testing for positivness, as the check in itself will slow
everything down.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#40 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEp7DCmLZNiovqbkRT-pj707yCo2UVy4ks5qiwTCgaJpZM4Jq72m
.

@alexandrebarachant
Copy link
Collaborator

To the best of my knowledge, this error is always fixed by regularization so it's safe to catch the ValueError and replace the error message.

@nbara what do you think of :

ValueError: Covariance matrices must be positive definite. Add covariance regularization to avoid this error

Would it be helpful ?
@kingjr other suggestion

@kingjr
Copy link
Contributor

kingjr commented Aug 23, 2016

LGTM, perhaps add a isnan check before fitting to ensure this is indeed the
source of the error (if not already done ?)

Le 23 août 2016 16:44, "alexandre barachant" notifications@github.com a
écrit :

To the best of my knowledge, this error is always fixed by regularization
so it's safe to catch the ValueError and replace the error message.

@nbara https://github.com/nbara what do you think of :

ValueError: Covariance matrices must be positive definite. Add covariance
regularization to avoid this error

Would it be helpful ?
@kingjr https://github.com/kingjr other suggestion


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#40 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEp7DLqmQY7izWFgD6Zophd7nnO5uEn3ks5qiwdNgaJpZM4Jq72m
.

@alexandrebarachant
Copy link
Collaborator

The nan is in the scipy function, so it's hard to catch. I will open a PR today.

@alexandrebarachant
Copy link
Collaborator

hum, i can also remove the check_finite in the scipy function and add my own test. let see.

@kingjr
Copy link
Contributor

kingjr commented Aug 23, 2016

I mean check that X doesn't contain any nan in the first place, I suppose
it would throw a similar error, although that d need to be checked (sorry I
m in transit)

Le 23 août 2016 16:49, "alexandre barachant" notifications@github.com a
écrit :

The nan is in the scipy function, so it's hard to catch. I will open a PR
today.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#40 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEp7DJydrB1FZvPKG4TU4V8m1eDhkUKGks5qiwh0gaJpZM4Jq72m
.

@alexandrebarachant
Copy link
Collaborator

This check has to be done at the lowest level possible i.e. in the function _matrix_operator in utils.base. This function is called a huge amount of times so i would like to avoid making unnecessary check that will slow down the whole toolbox.

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