visbrain icon indicating copy to clipboard operation
visbrain copied to clipboard

Use MNE for data loading, filtering and resampling

Open raphaelvallat opened this issue 4 years ago • 10 comments

Hi all!

Following on issue https://github.com/EtienneCmb/visbrain/issues/68, I think a nice way to limit bugs in Visbrain and simplify the code would be to use MNE instead of our custom functions to 1) read EEG files (e.g. EDF, BrainVision...), 2) apply filtering operations and 3) downsample the data.

For 1) users could now use either a NumPy array or an MNE Raw object. In addition, we would still keep the Elan and Micromed format since they are not yet supported by MNE.

What does everyone think?

Thanks

raphaelvallat avatar Mar 24 '20 16:03 raphaelvallat

I think it would be a smart move. And I say this not just because of my affiliation to MNE :)

jasmainak avatar Mar 24 '20 16:03 jasmainak

I agree, but the doc should reflect the dependence to MNE when using the sleep module.

BTW ciao Mainak 😁

EtienneCmb avatar Mar 24 '20 17:03 EtienneCmb

Thanks for your quick replies guys! Indeed this would require a lot of changes in the doc and examples.

One other thing that I'm not sure is whether we should still allow users to specify a path to an EDF file (e.g. myfile.edf) that would then be internally loaded by Visbrain using mne.io.read_raw_edf, or, if to save us some trouble, we just force the users to first create a Raw object and then pass this object directly to Visbrain? I would favor the latter -- even though it means more work for the users -- for the simple reason that most of the issues that we have on Gitter now are not directly related to Visbrain per se but rather to data loading...

raphaelvallat avatar Mar 24 '20 17:03 raphaelvallat

What does Visbrain need? If you just need a numpy array, I would suggest that users to go even one step further and pass the numpy array instead of the MNE object.

raw = mne.io.read_raw_edf(fname)
data = raw[:][0]

You can avoid the MNE dependency that way. It's two lines extra for users but big win for everyone.

jasmainak avatar Mar 24 '20 17:03 jasmainak

We also need the channel names, sampling frequency and meas_date. To be honest, I don't think that adding MNE as a formal dependency of Visbrain is an issue. And I prefer to work with mne.Raw object than NumPy array for EEG data so I would prefer to keep an option to pass the Raw object directly.

raphaelvallat avatar Mar 24 '20 17:03 raphaelvallat

okay sounds good. Probably better to just stick to raw object then. For IO stuff, a simple pip install should suffice.

jasmainak avatar Mar 24 '20 17:03 jasmainak

might this also be a good moment to include loading channels of different frequency?

skjerns avatar Mar 26 '20 08:03 skjerns

Hi @skjerns,

  1. How do you typically load EDFs files with channels of different frequencies? I don't think that MNE allows that (?).

  2. This may require some important change because the shape of the arrays is gonna be different which means we need to pass a list of arrays and a list of associated sampling frequencies, which will be hard to integrate into the current API. I think it would make more sense to ask the users to resample all their channels to a common sampling frequency prior to calling Visbrain. We can add a snippet in the documentation to show how to do that.

Thanks

raphaelvallat avatar Mar 26 '20 15:03 raphaelvallat

hey @raphaelvallat

  1. this was fixed a while ago, I just tested it and it seems to work with the current release
  2. might it be an idea to automate the resampling process internally in mne?

usually it only affects a handfull of channels, e.g. ecg, accelerometer, ambient light, etc... for scientific EEG systems this is usually not the case, but most medical PSGs record with different sampling frequencies.

Adding to the mne discussion: How about switching to MNE for data loading, but keep everything else as it is for now? As @jasmainak recommended. this would be fast&easy to implement. in a next step the dependency could then be switched over to use raw, with all the updates that are needed in the docs etc

skjerns avatar Oct 11 '20 08:10 skjerns

Hi @skjerns, I didn't know that MNE now supports loading channels of different sampling frequencies, I'll go and check that right away. And your plan sounds great, I'm all in for an MNE-based data loading in Visbrain. We can implement MNE-based resampling and filtering in future releases.

raphaelvallat avatar Oct 11 '20 16:10 raphaelvallat