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

allow lazy preproc #164

Merged
merged 2 commits into from Sep 23, 2020
Merged

allow lazy preproc #164

merged 2 commits into from Sep 23, 2020

Conversation

robintibor
Copy link
Contributor

Only load data if preprocessor needs it

@robintibor
Copy link
Contributor Author

robintibor commented Sep 3, 2020

one main use case here is that for mne crop function, otherwise all data is loaded before cropping, clearly undesirable

@@ -33,13 +33,21 @@ def __init__(self, fn, **kwargs):
self.kwargs = kwargs

def apply(self, raw_or_epochs):
try:
self._try_apply(raw_or_epochs)
Copy link
Collaborator

@agramfort agramfort Sep 3, 2020

Choose a reason for hiding this comment

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

there is something easier. You can check the preload attribute of the instance.

if not raw_or_epochs.preload:
     raw_or_epochs.load_data()

Copy link
Contributor Author

@robintibor robintibor Sep 4, 2020

Choose a reason for hiding this comment

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

Maybe I should make the comment clearer:
Some mne functions do not need the data to be loaded (e.g., mne crop), and in this case, we want to apply the function without loading the data. I am not aware of any way to know beforehand if the function will need the data to be loaded, hence the try/except.
Before what happened was:

  • I wanted to crop out only a small part of the recording using mne crop, and then apply other preprocessings next
  • Before mne crop was applied, in our previous Braindecode code (the code changed here), the whole data was loaded unnecessarily
  • This makes it substantially slower than in the code now proposed (I did perf-comparisons)

Will clarify the comment in the code

@robintibor robintibor merged commit e5e7439 into braindecode:master Sep 23, 2020
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.

None yet

2 participants