SpeechBrain EEG by ddavidebb · Pull Request #975 · speechbrain/speechbrain

ddavidebb · 2021-09-08T17:49:49Z

This is the first commit for a motor imagery decoding recipe based on a benchmark motor imagery dataset (BNCI2014001).
README and other minors are missing.

mravanelli · 2021-09-08T23:03:55Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/MOABB_dataio_iterators.py

@@ -0,0 +1,409 @@
+import numpy as np


Please, add a short docstring that explain what we are doing here

mravanelli · 2021-09-08T23:05:10Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/MOABB_dataio_iterators.py

+
+
+class WithinSession(object):
+    """Within Session """


Here you should improve the docstring with a better description of what we are doing + description of the arguments (see other docstrings)

mravanelli · 2021-09-08T23:05:59Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/MOABB_dataio_iterators.py

+        self.iterator_tag = "within-session"
+
+    def prepare(self, dataset):
+        if not (isinstance(dataset, BaseDataset)):


Please, add a docstring + description of input/output arguments

mravanelli · 2021-09-08T23:06:25Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/MOABB_dataio_iterators.py

+
+
+class CrossSession(object):
+    """Cross Session """


Improve docstring (like above)

mravanelli · 2021-09-08T23:06:35Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/MOABB_dataio_iterators.py

+
+
+class LeaveOneSubjectOut(object):
+    """Leave one subject out"""


Improve docstring (like above)

mravanelli · 2021-09-08T23:13:00Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/train.py

+if __name__ == "__main__":
+    argv = sys.argv[1:]
+    # Temporary switching off deprecation warning from mne
+    import warnings  # noqa


Is that needed? I don't use mne used and I think we solved the warning issue in the previous recipe, right?

mravanelli · 2021-09-08T23:14:30Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/train.py

+    # to run on a subset of subjects: moabb_dataset.subject_list = [1, 2, 3, 4]
+    moabb_dataset.download(path=hparams["data_folder"])
+    moabb_paradigm = MotorImagery(
+        n_classes=4,


The number of classes seems hardcoded it. Can we put in the yaml file?

mravanelli · 2021-09-08T23:15:51Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/train.yaml

@@ -0,0 +1,144 @@
+###################################################


Please, write the header. Here is the place where I would mention the EEGNET

mravanelli · 2021-09-08T23:16:40Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/train.yaml

+
+# DIRECTORIES
+data_folder: !PLACEHOLDER  #'/path/to/MOABB_BNCI2014001'
+output_folder: '/home/prometheus/Documents/results/MOABB_BNCI2014001' #!ref results/MOABB_BNCI2014001/<seed>


I think this should be output_folder: results/MOABB_BNCI2014001/<seed>. Otherwise, it is hardcoded.

mravanelli · 2021-09-08T23:18:41Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/train.yaml

+p_drop: 0.5  # dropout rate
+padding_mode: 'constant'
+
+layer0: !new:speechbrain.nnet.CNN.Conv2d


An alternative here would be to define the model in lobes (e.g. speechbrain/lobes/EEGNET.py). However, defining it every time doesn't look that bad to me in the end.

mravanelli · 2021-09-08T23:39:36Z

Thank you @ddavidebb , great job! This looks already in a good shape.
Beyond the punctual comments done above, let me share a couple of additional suggestions:

It looks like train.py is dataset-specific (BNCI2014001). However, I think the code should be easily made dataset-independent. For instance, instead of importing the library at the beginning, you can import it right after reading the hparams file. You can add a variable like in the yaml file (e.g., dataset="moabb.datasets.BNCI2014001") and import the library from this string using importlib like:

moabb_dataset= importlib.import_module(hparams["dataset"])
moabb_dataset = moabb_dataset()

e.g, https://www.devdungeon.com/content/import-python-module-string-name

If we are able to make the train.py dataset independent we can share it across all the datasets that we would like to address. Not sure about that, but maybe we can even create a single train.py for all the MOABB tasks (i.e., not only for Motor imagery detection). What do you think?

The MOABB_dataio_iterators.py looks something in common to all the MOABB datasets. One possible approach here (already used in other recipes for the data preparation part) could be to put the script in recipes/MOABB/ and add a symboling link in recipes/MOABB/MOABB/MotorImagery_decoding/BNCI2014001/

fpaissan · 2021-09-09T14:26:22Z

recipes/MOABB/MotorImagery_decoding/BNCI2014001/MOABB_dataio_iterators.py

+    return standardized
+
+
+def nth(iterable, n, default=None):


I think this is used on the StratifiedKFold splits...
don't we want to split the data such that we don't have the same sessions of the same subject on train and test?
I think there's a data leakage problem if we do like this. In fact, you already "know" your subject from the train set.
Maybe this is implemented somewhere else and I am just missing it!

…into eeg_decoding

…in parallel

…ile, added baselines, added EEGConformer, removed DeepConvNet and LMDA

… hparam space of ShallowConvNet and EEGConformer

mravanelli · 2023-10-02T18:36:45Z

I'm now going to merge the content of this PR in develop and open a new one in speechbrain/benchmarks with the EEG recipes.

ddavidebb added 4 commits September 8, 2021 19:29

first commit moabb motor imagery

f2ce000

initial commit MOABB motor imagery

9064d14

initial motor imagery commit

6d0b09e

First commit Motor imagery decoding

780c28b

ddavidebb requested a review from mravanelli September 8, 2021 17:50

ddavidebb added enhancement New feature or request work in progress Not ready for merge labels Sep 8, 2021

fixed linters

8bd5251

mravanelli requested changes Sep 8, 2021

View reviewed changes

mravanelli assigned ddavidebb Sep 8, 2021

fpaissan reviewed Sep 9, 2021

View reviewed changes

ddavidebb and others added 18 commits October 1, 2021 17:08

Reviewed after first round of revision

f058418

Removed outdated files

24f3704

Reviewed code after second round of revision

461abf3

reviewed README

78f6153

reviewed README

1177330

solved checkpoint saving issue + test on single best metrics

9d6861d

fixed linters

5b59e2e

fixed linters

0f78172

Merge branch 'develop' of https://github.com/speechbrain/speechbrain …

d1b2a42

…into eeg_decoding

fixed CNN transpose issue

11dff42

script for results aggregation

b0f6e2b

add docstring and parameter for result dir parsing

db37c79

fixed cross-session

c2a7f7e

direct results folder

bd39d7e

fixed linters

654399c

passing desired metrics as parameters

5706c87

tentive fix to nested dictionary

12e4679

small fixed in parsing and README

082db17

ddavidebb and others added 26 commits August 14, 2023 07:52

Fixed read/write conflicts mne config file when training many models …

331741d

…in parallel

Added hparam files for other architectures

0f25d5b

Updated LMDA, forcing odd kernel size in depth attention

9ba76e3

Fixed activation in LMDA

6336200

Fixed issue in deepconvnet

1593cc4

Fixed issue with shallowconvnet

2f0f5f0

Fixed issue with lmda

8f70136

Added template yaml files

66633a0

small fix on orion flag

8a98401

extend fix to all files

7da9a95

fix style

4b09ff2

Merge remote-tracking branch 'upstream/develop' into eeg_decoding

ced2922

fix useless file

5e070a2

Added bash script to save yaml files, fixed issue with orion config f…

164f8fe

…ile, added baselines, added EEGConformer, removed DeepConvNet and LMDA

Fixed issue in yaml

3d39ccd

Fixed issue in baseline yaml

321c9f7

Removed baselines, fixes in code format of ShallowConvNet, changes in…

6fd881e

… hparam space of ShallowConvNet and EEGConformer

Updated yaml files after hparam tuning

221f2da

Updated EEGConformer

208bccb

Updated README.md

dcc29c7

Updated README.md

5b791fe

Merge remote-tracking branch 'upstream/develop' into eeg_decoding

9861876

fix linters

d0296f5

improve README

e412656

remove unnecesary folder

e8be915

remove files that will be added into speechbrain benchmark

e431763

mravanelli removed the work in progress Not ready for merge label Oct 2, 2023

mravanelli approved these changes Oct 2, 2023

View reviewed changes

mravanelli merged commit 48f5ddd into speechbrain:develop Oct 2, 2023



		class LeaveOneSubjectOut(object):
		"""Leave one subject out"""

		@@ -0,0 +1,144 @@
		###################################################

Conversation

ddavidebb commented Sep 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mravanelli commented Sep 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mravanelli commented Oct 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants