[MRG] USLEEP model #282

tgnassou · 2021-06-25T13:36:03Z

Add USleep model for sleep staging
Joint work with @l-omar-chehab

…taset` to return sequences of windows - adding modified sleep staging examples that work on sequences of EEG windows - adding a method `embed` to `SleepStagetChambon2018` class to reuse it as a feature extractor in the sleep staging on sequences examples - adding tests

… test - updating sleep_staging_sequences example to use sampler instead of SequenceDataset

…ecode into sequence-dataset a

- adding seq_target_transform property to BaseConcatDataset to transform the targets when extracting sequences of SequenceWindowsDataset - adding functionality in BaseConcatDataset to index with a list of indices, such that a sequence a windows and targets can be returned - fixing SleepPhysionet bug where cropping would cause an error with some recordings - changing hyperparameters of sleep staging example to improve performance in a few epochs - adding more representative splitting functionality in sleep staging example - using class weights for imbalanced classes in sleep staging example

…ecode into sequence-dataset Fusioner usleep + sequence dataset

…ecode into sequence-dataset

…ecode into sequence-dataset new working branch

hubertjb · 2021-07-08T15:57:58Z

I just had a chat with @tgnassou and @l-omar-chehab. There are three things that still need to be done in this PR:

Clarify what hyparameters were actually used in the USleep paper. The paper, the Supplementary Materials and the keras implementation all have different descriptions of how the numbers of convolutional channels are computed. A quick summary:
- Paper (p.8): c_t = floor(c_{t-1} * np.sqrt(2)); c_1 = 5
- Suppl. (p.32): c_t = floor(c_{t-1} * np.sqrt(1.67)); c_1 = 6 (doesn't seem to fit with the values in the table)
- Keras : c_t = floor(h_t * np.sqrt(2)) where h_t = floor(h_{t-1} * np.sqrt(2)); h_1 = 5

@tgnassou and @l-omar-chehab will reach out to the authors to ask them which version they ended up using.

Make sure the model can be used with more than just one fixed input sampling rate. Right now the kernel size and maxpool size are hardcoded to avoid previously seen problems with the dimensions of layers. It looks like the actual problem is that the maxpool size and the depth size (i.e. number of blocks) need to be compatible. @tgnassou is looking at that.
We need another test that compares the dimensions of the model to those reported in the paper.

hubertjb · 2021-07-08T15:58:42Z

Also, to answer your previous comment @robintibor : crop_wake_mins actually shouldn't be decreased below 30 (it's the typical value used in the literature). I think at this point the data is really as small as it can get, unless we want to only use a few examples from the two sleep recordings that are used in the example.

robintibor · 2021-07-09T16:31:09Z

Maybe a clarification regarding examples as I see it @hubertjb
Obviously the nicest thing would be to have the best-performing pipeline as the examples, so people can directly use the code and expect great results and at the same time it is validated to run by the CI.
However, this would take too long on the CI, and in the end we decided that:

all decoding examples will run in CI
code should be as close as possible to good pipeline so users may not be confused and run suboptimal hyperparameters expecting good results
to ensure fast runs on CI, we drastically reduce data and epochs, even if it makes the example nonsensical as it is and clearly note that these are not useful values, indicating what are the useful values

This way, we can make sure the code always runs, is fast enough on CI, and should prevent users from mistakenly adopting suboptimal pipelines. And it is acceptable if the example due to reduced data and reduced number of examples does not learn anything as is (e.g., our trialwise decoding example (https://braindecode.org/master/auto_examples/plot_bcic_iv_2a_moabb_trial.html) is barely above chance on train and exactly at chance on valid at the end of training).

Otherwise, with examples that are "more realistic" but with suboptimal hyperparameters, we have seen users adopt suboptimal hyperparameters in their training and expect good results.

tgnassou · 2021-07-16T13:08:36Z

This PR is ready to merge

agramfort · 2021-07-05T10:03:13Z

braindecode/models/usleep.py

+        depth = 10
+        time_conv_size = 9  # 0.09s at sfreq = 100 Hz
+        max_pool_size = 2   # 0.02s at sfreq = 100 Hz
+        n_time_filters = 5


if we expose this parameter in the init we'll need to add a test to make sure it works for different values no? No strong feeling here. @hubertjbanville thoughts?

agramfort · 2021-07-17T06:40:10Z

examples/plot_sleep_staging_usleep.py

@@ -209,6 +209,7 @@
    with_skip_connection=True,
    n_classes=n_classes,
    input_size_s=input_size_samples / sfreq,
+    apply_softmax=True


you need to use nn.CrossEntropyLoss here as you use class-weights? if so can you add a comment
so it's clear why you need to change a default parameter in a first example?

what about these questions @tgnassou ? please do add some comment about the depth=12 line before the model creation, so it shows up clearly in the example doc why we reduce here. I am also confused about apply_softmax=True and using nn.CrossEntropyLoss as doc of cross entropy loss states:
https://pytorch.org/docs/stable/generated/torch.nn.CrossEntropyLoss.html

This criterion combines LogSoftmax and NLLLoss in one single class. [...] The input is expected to contain raw, unnormalized scores for each class.```

Did you see this comment here @hubertjb ?

agramfort · 2021-07-17T06:41:16Z

examples/plot_sleep_staging_usleep.py

+
+.. warning::
+    The example is written to have a very short excecution time.
+    This number of epochs is here too small and very few recordings are used.


Suggested change

This number of epochs is here too small and very few recordings are used.

The number of epochs is here too small and very few recordings are used.

agramfort · 2021-07-17T06:42:38Z

examples/plot_sleep_staging_usleep.py

+.. warning::
+    The example is written to have a very short excecution time.
+    This number of epochs is here too small and very few recordings are used.
+    To obtain competitive results you need to use more data and more epochs.


Given what you observe can you be more specific? How many epochs will lead to
what accuracy (roughly). Something like : Taking for example at least 30 recordings
to training and doing 40 epochs should lead to a balanced accuracy close to 0.8.

can you also put the same warning in plot_sleep_staging.py ?

Yes and also add the changing of the depth parameter, in case that was for execution time @tgnassou

robintibor · 2021-07-20T10:08:54Z

See comments by me and alex, and also @tgnassou I invited you to be maintainer, then you don't have to always wait for me to approve workflows. could you accept this invitation?

robintibor · 2021-08-09T13:24:15Z

So I think the loss comment from me above #282 (comment) definitely needs addressing. Is somebody still active on it or do we need to search somebody @hubertjb @tgnassou @agramfort

tgnassou · 2021-08-09T13:30:30Z

I am in holidays, that's why i'm not very active these days. But I will continue the work in september! Is it ok for you @robintibor ?

robintibor · 2021-08-09T16:41:37Z

ok my plan was to have a release ~end of august, but maybe we can shift to midseptember anyways, then this one could be inside as well

hubertjb · 2021-08-09T17:46:13Z

I'm still working on it and should have results soon @robintibor

… number of filters of each layer, first conv layer in decoder blocks) - defining kernel size based on sfreq

…an even number - adding test for usleep number of parameters

agramfort · 2021-08-12T09:26:34Z

@hubertjbanville nice ! can you just share a learning / convergence curve before we merge this? just to check that it's learning well and reaching good performance eg vs. Chambon et al. 🙏

hubertjb · 2021-08-13T02:40:54Z

Yes, I'll share that asap!

So far I can say that the balanced sequence sampler does seem to speed up learning a bit. The USleep model, especially in its depth=12 version, is massively overparametrized though, especially given a smallish dataset like Sleep Physionet. Reducing the depth and adding some weight decay helps, but the SleepStagerChambon2018 learns much more quickly and so far reaches higher test performance.

Also I did see a minor improvement in validation performance when combining softmax + CrossEntropyLoss, but so far not as much as what @tgnassou reported.

More to come soon!

hubertjb · 2021-09-07T03:39:11Z

Trying to conclude on this @robintibor @agramfort @tgnassou

U-Sleep is much harder to train on the "small-ish" SleepPhysionet. I found some configurations that could somewhat learn more quickly (e.g. model depth=8, weight decay=5e-5, learning rate=1e-5), but they never go much above 50% test balanced accuracy. SleepStagerChambon2018 in contrast can easily reach >70% in under 100 epochs. Using really low learning rates (e.g. 1e-7 like in the paper) does yield very smooth learning curves, but we would have to train models for a lot more than the 100 epochs I tested on to get interesting performance.
The balanced sampler seems to help quite a lot. The models learn faster and reach slightly better test performance. Here are some learning curves obtained with SleepStagerChambon2018:

The self-ensembling also does help, with gains of 5-10% balanced accuracy in my limited experiments on (overfitting) U-Sleep.

Overall, I'm confident the model implementation is accurate. I went through the original keras implementation and made sure the dimensions and the number of parameters were exactly the same (there are now unit tests for these). The model does not learn as well on SleepPhysionet, but my guess is this is an issue with the scale of the data and the training budget rather than the model itself.

agramfort · 2021-09-07T05:36:13Z

Great summary. I suggest to merge. Thanks @hubertjb for finishing this and @tgnassou and @l-omar-chehab for the ground work

tgnassou · 2021-09-07T06:42:23Z

Very nice ! Thank you @hubertjb !

agramfort · 2021-09-07T12:55:43Z

@robintibor the green button is yours !

robintibor · 2021-09-09T10:08:41Z

There were some conflicts now due to other PR merge, which I have resolved.
More importantly @hubertjb , it seems the comment regarding applying softmax and using crossentropyloss has not been addressed I think (#282) ? Crossentropyloss will apply softmax again, so it is not correct what is happening there I think... Maybe even apply_softmax=False could already solve it? Or do I have some misunderstanding? :)

hubertjb · 2021-09-09T18:56:07Z

Thanks @robintibor for fixing the conflicts and for pointing that out! I forgot to change that line - it should indeed be apply_softmax=False.

robintibor · 2021-09-10T15:35:22Z

Great stuff, thanks @tgnassou @l-omar-chehab @hubertjb , merged now

agramfort · 2021-09-11T08:11:37Z

awesome team effort @tgnassou @l-omar-chehab @hubertjb 🚀 🚀 🚀 🚀 👏 👏 👏 👏

“l-omar-chehab“ and others added 30 commits June 21, 2021 18:21

Layed out plans for USleep architecture in torch.

3d95159

ignoring flake8 lambda error

ee73820

First iteration on USleep written in torch.

9f1ad92

Merge branch 'usleep' of https://github.com/braindecode/braindecode

d226140

Corrected some dims in USleep net.

4f7c7cf

fix shape

08aa2f7

add usleep-keras

ca3aa87

refactored some code

486dc66

applying PR comments

d651ff3

- adding SequenceSampler to sample sequences of consecutive windows +…

12de166

… test - updating sleep_staging_sequences example to use sampler instead of SequenceDataset

remove unused target_transform

92a6c5a

Added some documentation + modularity to our implementation of USleep.

c2a3520

Added the script with refactored USleep code.

7848ad4

updated ulseep and add exemple

b8b4fe2

update usleep_wip.py

5cad5eb

Delete usleep_wip.py

5e3dd03

Add files via upload

b8d4a10

Merge branch 'sequence-dataset' of https://github.com/hubertjb/braind…

57b3a9f

…ecode into sequence-dataset a

Modified USleep modular breakdown.

d691c5a

Delete usleep_wip.py

39ce658

Add files via upload

d62a33c

Trying to push changes: USleep + new classifier.

76fac38

Adapted dimensions for input = sequence of windows.

3932077

Merge branch 'sequence-dataset' of https://github.com/hubertjb/braind…

c5c912c

…ecode into sequence-dataset Fusioner usleep + sequence dataset

added script comparing conv1d and conv2d

e8e28de

Merge branch 'sequence-dataset' of https://github.com/Tgnassou/braind…

a9cbf3d

…ecode into sequence-dataset

Merge branch 'sequence-dataset' of https://github.com/Tgnassou/braind…

b07d806

…ecode into sequence-dataset new working branch

modify Hubert's sequence-of-windows example for USleep.

1dff4ef

Theo Gnassounou added 3 commits July 12, 2021 10:18

FIX:parameters

dcb0795

FIX: numbers of filters

cb64681

add apply_softmax=True

eaf42bd

tgnassou changed the title ~~[WIP] USLEEP model~~ [MRG] USLEEP model Jul 16, 2021

agramfort reviewed Jul 17, 2021

View reviewed changes

hubertjb mentioned this pull request Jul 24, 2021

[MRG] Random sampler #295

Merged

hubertjb added 2 commits August 10, 2021 21:35

- fixing discrepancies with keras version (default complexity factor,…

b10aaf6

… number of filters of each layer, first conv layer in decoder blocks) - defining kernel size based on sfreq

- adding argument ensure_odd_conv_size to adapt conv_size if it is …

7990a8c

…an even number - adding test for usleep number of parameters

Merge branch 'master' into sequence-dataset

dfac915

setting apply_softmax to False to work with CrossEntropyLoss

eaf5818

robintibor merged commit fa6042b into braindecode:master Sep 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] USLEEP model #282

[MRG] USLEEP model #282

tgnassou commented Jun 25, 2021

hubertjb commented Jul 8, 2021 •

edited

Loading

hubertjb commented Jul 8, 2021

robintibor commented Jul 9, 2021

tgnassou commented Jul 16, 2021

agramfort Jul 5, 2021

agramfort Jul 17, 2021

robintibor Jul 20, 2021

robintibor Aug 31, 2021

agramfort Jul 17, 2021

agramfort Jul 17, 2021

agramfort Jul 17, 2021

robintibor Jul 20, 2021

robintibor commented Jul 20, 2021

robintibor commented Aug 9, 2021

tgnassou commented Aug 9, 2021

robintibor commented Aug 9, 2021

hubertjb commented Aug 9, 2021

agramfort commented Aug 12, 2021

hubertjb commented Aug 13, 2021

hubertjb commented Sep 7, 2021

agramfort commented Sep 7, 2021

tgnassou commented Sep 7, 2021 •

edited

Loading

agramfort commented Sep 7, 2021

robintibor commented Sep 9, 2021

hubertjb commented Sep 9, 2021

robintibor commented Sep 10, 2021

agramfort commented Sep 11, 2021

	This number of epochs is here too small and very few recordings are used.
	The number of epochs is here too small and very few recordings are used.

[MRG] USLEEP model #282

[MRG] USLEEP model #282

Conversation

tgnassou commented Jun 25, 2021

hubertjb commented Jul 8, 2021 • edited Loading

hubertjb commented Jul 8, 2021

robintibor commented Jul 9, 2021

tgnassou commented Jul 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robintibor commented Jul 20, 2021

robintibor commented Aug 9, 2021

tgnassou commented Aug 9, 2021

robintibor commented Aug 9, 2021

hubertjb commented Aug 9, 2021

agramfort commented Aug 12, 2021

hubertjb commented Aug 13, 2021

hubertjb commented Sep 7, 2021

agramfort commented Sep 7, 2021

tgnassou commented Sep 7, 2021 • edited Loading

agramfort commented Sep 7, 2021

robintibor commented Sep 9, 2021

hubertjb commented Sep 9, 2021

robintibor commented Sep 10, 2021

agramfort commented Sep 11, 2021

hubertjb commented Jul 8, 2021 •

edited

Loading

tgnassou commented Sep 7, 2021 •

edited

Loading