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

Add BEETL data to MOABB for benchmarking #675

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

Samuel-Boehm
Copy link
Contributor

Hi MOABB team,

I'm a PhD student working on BCI benchmarking and would like to contribute code to integrate the BEETL dataset into MOABB. Before proceeding with the review, I want to be transparent that I am not the owner or license holder of the original BEETL dataset - I'm simply contributing code to make this publicly available dataset accessible through MOABB for benchmarking purposes.

The dataset is publicly available through Figshare and was published as part of a BCI competition. I've implemented the loader following MOABB's conventions and patterns as good as I can. I basically followed what is done for Stieger2021 as I saw that they also uploaded the data to Figshare.

Key implementation details:

  • Dataset split into leaderboard and final evaluation phases (This might be confusing first but I wanted to stick to the original split as it was during the competition.
  • Handles two different data formats (Dataset A and B) with different sampling rates and channel configurations
  • Proper handling of labels and trial information
  • After the competition was done a .txt with the hidden labels was released, this is also downloaded and added to the data here.

Important Implementation Note:
In the current implementation, I concatenate all runs/races for each subject into a single continuous Raw object. This differs from the original data structure where trials were stored separately. I made this choice to align with MOABB's typical usage patterns, but I'm open to discussion if this should be handled differently.

I would greatly appreciate:

  1. Your review of the code quality and adherence to MOABB standards
  2. Guidance on whether it's appropriate to include this dataset given I'm not the original data holder

Greetings from Germany, Merry Christmas and a Happy New Year!
Samuel

@sylvchev
Copy link
Member

sylvchev commented Jan 20, 2025

Thanks, that is a really nice addition!
For the dataset split, the solution you proposed is perfectly fine.

Copy link
Member

@sylvchev sylvchev left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, I'm not sure about the downloader changes, could you explain?

@@ -206,30 +206,39 @@ def fs_issue_request(method, url, headers, data=None, binary=False):

def fs_get_file_list(article_id, version=None):
"""List all the files associated with a given article.

Copy link
Member

Choose a reason for hiding this comment

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

Please avoid changing docstring formatting, it is done to generate automatically the doc on the website, you could revert those two line suppression.

all_files = []
page = 1

while True:
Copy link
Member

Choose a reason for hiding this comment

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

Why did you need to change the downloader function for this dataset? There is 2 items max for this dataset.
If you want to address a specific issue for another dataset, please open another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, that is a fix I wrote for the Stieger2021 dataset and does not belong here! I branched my other developing branch, not knowing this change is still in there. It should be the original function, Ill change it back.

Accidentally copied fs_get_file_list from another branch. Reverted to default version.
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.

3 participants