Skip to content

[FIX] Making the MOABB dataset work with BIDS cache - #694

Merged
bruAristimunha merged 9 commits into
braindecode:masterfrom
bruAristimunha:moabb-cache-dataset
Feb 28, 2025
Merged

[FIX] Making the MOABB dataset work with BIDS cache#694
bruAristimunha merged 9 commits into
braindecode:masterfrom
bruAristimunha:moabb-cache-dataset

Conversation

@bruAristimunha

Copy link
Copy Markdown
Collaborator

Code to test:

from braindecode.datasets import MOABBDataset
from braindecode.preprocessing import create_windows_from_events
cache_config = dict(
    use=True,
    path="~/mne_data",
    save_raw=True,
    save_epochs=False,
    save_array=False,
    overwrite_raw=False,
    overwrite_epochs=False,
    overwrite_array=False,
)

dataset = MOABBDataset("AlexMI", subject_ids=[1], dataset_load_kwargs={'cache_config': cache_config})

windows = create_windows_from_events(dataset)

@codecov

codecov Bot commented Feb 17, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 50.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 87.17%. Comparing base (f775002) to head (4463268).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #694      +/-   ##
==========================================
- Coverage   87.25%   87.17%   -0.09%     
==========================================
  Files          76       76              
  Lines        6835     6846      +11     
==========================================
+ Hits         5964     5968       +4     
- Misses        871      878       +7     

@bruAristimunha
bruAristimunha requested review from PierreGtch and sylvchev and removed request for sylvchev February 17, 2025 17:09
Comment thread docs/whats_new.rst Outdated
Comment thread braindecode/datasets/moabb.py Outdated
Comment on lines +59 to +65
# find events from the stim channel
stim_channels = mne.utils._get_stim_channel(None, raw.info, raise_error=False)
if len(stim_channels) > 0:
# returns an empty array if none found
events = mne.find_events(raw, shortest_event=0, verbose=False)
else:
events, _ = mne.events_from_annotations(raw, verbose=False)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# find events from the stim channel
stim_channels = mne.utils._get_stim_channel(None, raw.info, raise_error=False)
if len(stim_channels) > 0:
# returns an empty array if none found
events = mne.find_events(raw, shortest_event=0, verbose=False)
else:
events, _ = mne.events_from_annotations(raw, verbose=False)
events, _ = mne.events_from_annotations(raw, verbose=False)

I think we should always get the events using the annotations (with NeuroTechX/moabb#696, the annotations are always set in raw objects obtained from moabb)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

For some strange reason, it does not happen in all the datasets. I can try to run without this and fix this on the moabb side.

@bruAristimunha

Copy link
Copy Markdown
Collaborator Author

Hey Pierre,

It is falling already with the fake dataset, as I need this to run some stuff. For me, we merge this PR and open one issue on moabb to solve this in the future. It is a problem on the moabb side.

@PierreGtch PierreGtch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@bruAristimunha I'm not sure I understand why this PR is now needed; what changed on the MOABB/MNE side?
But let's merge if you think it is best.
We should take a deep look at versions compatibility with MOABB before making the next braindecode release

Comment thread braindecode/preprocessing/windowers.py Outdated
@bruAristimunha

Copy link
Copy Markdown
Collaborator Author

I will take some time to merge this pr, don't worry

@bruAristimunha
bruAristimunha merged commit 7071a71 into braindecode:master Feb 28, 2025
@bruAristimunha
bruAristimunha deleted the moabb-cache-dataset branch February 28, 2025 17:05
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.

2 participants