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

EDF writer #5

Open
yjmantilla opened this issue Jun 4, 2021 · 3 comments
Open

EDF writer #5

yjmantilla opened this issue Jun 4, 2021 · 3 comments
Labels
3rd-party involved We depend on some other part to complete this low-priority We will fix these ones one day

Comments

@yjmantilla
Copy link
Owner

yjmantilla commented Jun 4, 2021

So I have been delving into this edf conversion topic. What I have found is the following:

Robert Oostenveld and Phillip Alday did a pure python edf writer: pyedf . It is in python2 but has a branch on python3 and also the following repo https://github.com/bsandeepan95/pyedf . It also seems slower (expected) than pyedflib (bids-standard/pyedf#6)

Apparently pyedf has not been integrated to mne because it adds maintainance overhead while making relatively little difference in usability

Why is this important?

Our design has mne-bids as the bids writer, that is, we just infer the necessary arguments and the "CONVERT THEM" module does the job with mne-bids inside. So as it is now, we are constrained by what mne-bids accepts as outputs:

format : 'auto' | 'BrainVision' | 'FIF' Controls the file format of the data after BIDS conversion. If 'auto', MNE-BIDS will attempt to convert the input data to BIDS without a change of the original file format. A conversion to a different file format (BrainVision, or FIF) will only take place when the original file format lacks some necessary features. When a str is passed, a conversion can be forced to the BrainVision format for EEG, or the FIF format for MEG data.

So we have some options:

  • Suggest edf conversion (and do a PR) in mne-bids being the main argument that it is one of the recommended formats of bids. I guess this would be done with pyedf since it does not need compiled code.
  • Convert the output of mne-bids to edf by a postprocess code.
  • Don't support edf for the moment.

@stebo85 @civier @TomEmotion What do you think?

@civier
Copy link
Collaborator

civier commented Jun 4, 2021

Thanks @yjmantilla for investigating it. Very surprising given that edf is one of two recommended formats for BIDS.

I originally suggested edf over BrainVision because it is an open standard (see @TomEmotion comment in our meeting), but for the GSoC project, maybe we should only focus on BrainVision as the output format. It is still one of the recommended BIDS formats, so I don't see an issue with that. Adding edf support can be a longer-term goal, and ideally done by incorporating edf support into MNEbids.

Any thoughts mentors? Dave? @stebo85 @TomEmotion @aswinnarayanan @DavidjWhite33

@stebo85
Copy link
Collaborator

stebo85 commented Jun 6, 2021

Starting with BrainVision makes sense and later we could investigate how to best get things converted into EDF.

I think these problems should be solved upstream (in MNE?), but we should help if we have the time towards the end of the project:
pyedflib - I had a similar problem with cython and found a good workaround by compiling the cython code before execution on the target system -> by this we avoided distributing precompiled binaries that wouldn't be optimized for the hardware at hand (https://github.com/NeuroDesk/caid/blob/master/recipes/qsmxtbase/setup.py). The other option is to compile it with "-march=x86-64" and this will run on almost all CPUs (but slower than with specific instruction sets) -> https://github.com/NeuroDesk/caid/blob/master/recipes/tgvqsm/setup.py
pyedf - looks good, too - I wouldn't be too worried about the speed to start with - worth a try?

Cheers
Steffen

@yjmantilla
Copy link
Owner Author

So for now, I guess the conclusion is to use brainvision. Later if we have got time we can try to address the issue in MNE-BIDS through pyedf (why MNE-BIDS? I don't think the matter is relevant enough for MNE as a whole).

@yjmantilla yjmantilla added low-priority We will fix these ones one day 3rd-party involved We depend on some other part to complete this labels Jun 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd-party involved We depend on some other part to complete this low-priority We will fix these ones one day
Projects
None yet
Development

No branches or pull requests

3 participants