NeuroTechX / moabb

Mother of All BCI Benchmarks
https://neurotechx.github.io/moabb/
BSD 3-Clause "New" or "Revised" License
683 stars 177 forks source link

Paradigm picks channels could be wrong #227

Closed peara closed 3 years ago

peara commented 3 years ago

When using Paradigm to load data, it seems to disregard the order of channels. This could be huge as different datasets can use different order of channels. For example, these two will return the same:

MotorImagery(channels=['Fp1', 'Fz']).get_data(...)
MotorImagery(channels=['Fz', 'Fp1']).get_data(...)

The reason seems to be this line https://github.com/NeuroTechX/moabb/blob/develop/moabb/paradigms/base.py#L121 as using mne.pick_types do not preserve order of channels. I guess change to mne.pick_channels(ordered=True) should fix it.

Can anyone clarify this one as I don't see any test about picking channels.

sylvchev commented 3 years ago

Hi, thanks for raising this issue. Channel order was not a concern, explaining the lack of tests.

I think the rationale was to avoid computing multiple time the same results, so evaluations obtained on the same datasets/paradigm with same pipelines were considered equivalent if they used the same unordered set of electrodes. To build on your example, the results returned by a WithinSessionEvaluation on paradigm = MotorImagery(channels=['Fp1', 'Fz']) or on paradigm = MotorImagery(channels=['Fz', 'Fp1']) were considered equivalent. This is because most of the considered classification methods do not make use of the channel order.

Do you have in mind a case where the order of the channel passed in paradigm is important?

peara commented 3 years ago

Hi, thanks for your reply. Actually, I was not concern about channels' order until recently when I participated in the Beetl transfer learning competition. The competition uses moabb with 3 datasets. Because those datasets have different channels' order, if using Paradigm to load training data, the result would be wrong and many people may not recognize that.

sylvchev commented 3 years ago

Ok, got it. The channel order returned by the same paradigm for different dataset could vary depending on the channels order. Here is an example:

from moabb.datasets import Cho2017, BNCI2014001, PhysionetMI
from moabb.paradigms import MotorImagery
from moabb.datasets.utils import find_intersecting_channels

datasets = [Cho2017(), BNCI2014001(), PhysionetMI()]

common_channels, _ = find_intersecting_channels(datasets)
chans = common_channels[:3]
paradigm = MotorImagery(channels=chans)

for d in datasets:
    ep, _, _ = paradigm.get_data(dataset=d, subjects=[1], return_epochs=True)
    print(f"{d.code} channel order: {ep.info['ch_names']}")

and the results is:

Cho2017 channel order: ['Fz', 'C2', 'CP4']
001-2014 channel order: ['Fz', 'C2', 'CP4']
Physionet Motor Imagery channel order: ['C2', 'CP4', 'Fz']

I was misled by your example, as this is the correct behavior to return the same channel order when specifying channel list in paradigm.

Commonly, the channel order have no real impact on processing pipelines. That being said, I could push a fix very quickly.

peara commented 3 years ago

Thank you, please fix it. Your example was exactly is exactly what I mean. I'm sorry that my example was not clear.

sylvchev commented 3 years ago

Sorry, I did not want to be rude. It is nice that you made the effort to provide an example, with your explanation I could understood the problem, this is what matter.

I updated the code and pushed a new version of MOABB on pypi. The version 0.4.3 should be available soon. With a pip install -U moabb, you should get the newest version and the problem should be fixed on this version.

You could reopen this issue if needed.