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

Re-structuring the moabb core, implementing caching, creating tutorials, fixing some bugs and more #408

Merged
merged 93 commits into from
Aug 1, 2023

Conversation

PierreGtch
Copy link
Collaborator

@PierreGtch PierreGtch commented Jun 23, 2023

Closes #385
Closes #391 (BIDS conversion)

The goal of his PR is to save, on disk, a semi-processed version of the datasets to speed up their loading later.

The approach rn is to save the raw objects in a BIDS structure using mne_bids in EDF. We only apply the bandpass filters to the data before caching it. This way, the cached EDF files can then be read with preload=False and only the epochs are loaded in memory.

@PierreGtch
Copy link
Collaborator Author

PierreGtch commented Jun 24, 2023

TODO

@PierreGtch
Copy link
Collaborator Author

PierreGtch commented Jul 3, 2023

Comments of Sylvain during meeting of June 30:

  • merge all the caching parameters into one (for example a dict)
  • Use a pipeline to resample the raw objects

Do you have more comments @sylvchev ?

Copy link
Collaborator

@bruAristimunha bruAristimunha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @PierreGtch!
So fair is super okay, I reviewed 13 of 23 files, and I will need more time for the reviewing. I am directly changing the file to minimize your work.

moabb/datasets/bids_interface.py Outdated Show resolved Hide resolved
moabb/datasets/bids_interface.py Outdated Show resolved Hide resolved
moabb/datasets/bids_interface.py Outdated Show resolved Hide resolved
moabb/datasets/bids_interface.py Show resolved Hide resolved
moabb/datasets/bids_interface.py Outdated Show resolved Hide resolved
moabb/paradigms/base.py Outdated Show resolved Hide resolved
moabb/tests/paradigms.py Outdated Show resolved Hide resolved
docs/source/datasets.rst Outdated Show resolved Hide resolved
docs/source/paradigms.rst Outdated Show resolved Hide resolved
docs/source/whats_new.rst Outdated Show resolved Hide resolved
@PierreGtch
Copy link
Collaborator Author

Hey @PierreGtch!
So fair is super okay, I reviewed 13 of 23 files, and I will need more time for the reviewing. I am directly changing the file to minimize your work.

Perfect, thanks for directly updating the files

@bruAristimunha bruAristimunha changed the title Disk cache Re-structuring the moabb core, implementing caching, creating tutorials, fixing some bugs and more Aug 1, 2023
@bruAristimunha
Copy link
Collaborator

Hey @PierreGtch!

I REALLY liked how you defined the transformations for the basis of our paradigms, and I think it's cool how it was done! The new BID core structure is super good, I think the methodological design is clear, and all lines are clear in the context. I was not an easy PR to review haha

I have some questions about the amount of testing, some documentation comments, why it might not be obvious in some cases.
However, given the size and amount of things in the PR, all points that I think are important, either corrected directly or opened an issue for us to discuss in the future. That way, it doesn't block the merge of this PR.

Congratulations on all the effort. I think this will make our reading and writing bottleneck a lot easier!

@bruAristimunha bruAristimunha merged commit 4ab1d3c into NeuroTechX:develop Aug 1, 2023
7 checks passed
@bruAristimunha
Copy link
Collaborator

Thank you @PierreGtch :)

@PierreGtch
Copy link
Collaborator Author

No problem, It was fun to do 😁

PierreGtch added a commit to thijor/moabb that referenced this pull request Aug 18, 2023
bruAristimunha added a commit that referenced this pull request Aug 23, 2023
* add thielen2021 cvep dataset

* add cvep paradigm

* add thielen2021 cvep dataset tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* update documentation related to addition c-VEP

* Update cVEP paradigm according to the changes of #408

* change c-VEP paradigm to (potentially) multi-class

* add c-VEP paradigm tests

* add c-VEP banchmark tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* update c-VEP documentation

* remove redundant interval parameter from Thielen2021 dataset

* remove redundant tmin/tmax tests from BaseCVEP

* Add thielen2021 to datasets __init__.py

* Add default interval

* Complete scoring cases and add test

* Reformat events in FakeDataset code

* Fix coquille

* Update fake cVEP datasets parameters

* update documentation cvep

* Add link in dataset_summary.rst

* Refactoring the tests

* Updating pytest

* Fixing conflict with mne-bids and pytest

* Increase the codacov

* update documentation cvep

* split functionality for reuse and update documentation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* add channel locations thielen2021 from loc file

* Clarify paradigm doc (epoch level decoding)

* Fixing conflict

---------

Co-authored-by: Jordy Thielen <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: PierreGtch <[email protected]>
Co-authored-by: Pierre Guetschel <[email protected]>
Co-authored-by: bruAristimunha <[email protected]>
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 this pull request may close these issues.

Disk caching of preprocessing/transformation result
2 participants