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

Incorrect Interval for BNCI2014001 #532

Closed
dcwil opened this issue Dec 7, 2023 · 6 comments
Closed

Incorrect Interval for BNCI2014001 #532

dcwil opened this issue Dec 7, 2023 · 6 comments

Comments

@dcwil
Copy link

dcwil commented Dec 7, 2023

Someone with better knowledge of the codebase can maybe weigh in here but I think since #447 at least BNCI2014001 has been cutting trials without the shift for task onset/offset. Currently for BNCI2014_001 (the dataset I noticed this with) I think you need to set tmin=2 and tmax=6 in the paradigm to actually cut the correct trial. I assume by default it should tmin=0 should be relative to the dataset interval [2, 6] (so by default, it cuts the correct trial).

@dcwil
Copy link
Author

dcwil commented Dec 7, 2023

@bruAristimunha @PierreGtch Could you look at this/confirm? I think if left unnoticed by a user (aka me haha) this can have a pretty big impact on classification performance without throwing any errors

@PierreGtch
Copy link
Collaborator

Hi @dcwil, could you provide an example of code that changed behaviour since #447?

@dcwil
Copy link
Author

dcwil commented Dec 7, 2023

I'll try and find something, although I'm not sure what the intent is/was and I might have the PR wrong.

What I can say now is:
The events found by

events = mne.find_events(raw, shortest_event=0, verbose=False)
for BNCI2014001 are the fixation markers, so 2s before the actual task onset, hence the [2, 6] interval, but as far as I can tell the interval isn't applied anywhere in the prepro (if it is can you point me in the right direction?). If this is the intended functionality then the only thing that needs changing is stuff in examples that should probably use the interval to cut the trial:
tmin = 0
tmax = None

@PierreGtch
Copy link
Collaborator

I'not sure I understand where the problem is, [2,6] is the default interval in the BNCI2014_001 class:

interval=[2, 6],

In case the preprocessing is a bit unclear, do you know the paradigms have a make_process_pipelines() method?
For example:

from moabb.datasets import BNCI2014_001
from moabb.paradigms import MotorImagery
print(MotorImagery().make_process_pipelines(BNCI2014_001()))
[Pipeline(steps=[(<StepType.RAW: 'raw'>,
                 SetRawAnnotations(durations=4,
                                   event_id={'feet': 3, 'left_hand': 1,
                                             'right_hand': 2, 'tongue': 4})),
                (<StepType.RAW: 'raw'>,
                 FunctionTransformer(func=operator.methodcaller('filter', l_freq=8, h_freq=32, method='iir', picks='eeg', verbose=False))),
                (<StepType.EPOCHS: 'epochs'>,
                 Pipeline(steps=[('epoching',
                                  Pipeline(steps=[('forkpip...
                                                   RawToEpochs(baseline=None,
                                                               event_id={'feet': 3,
                                                                         'left_hand': 1,
                                                                         'right_hand': 2,
                                                                         'tongue': 4},
                                                               tmax=6,
                                                               tmin=2.0))]))])),
                (<StepType.ARRAY: 'array'>,
                 ForkPipelines(transformers=[('X',
                                              Pipeline(steps=[('get_data',
                                                               FunctionTransformer(func=operator.methodcaller('get_data'))),
                                                              ('scaling',
                                                               FunctionTransformer(func=operator.methodcaller('__mul__', 1000000.0)))])),
                                             ('events', EpochsToEvents())]))])]

Printing this scikit learn Pipeline is not very pretty but you can inspect it in detail.
Here you see the interval for epoching is [2-6]s.

@dcwil
Copy link
Author

dcwil commented Dec 8, 2023

Ah i see - sorry for the confusion, I was a little too hasty!

@dcwil dcwil closed this as completed Dec 8, 2023
@PierreGtch
Copy link
Collaborator

no problem @dcwil 😌

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

No branches or pull requests

2 participants