neuropsychology / NeuroKit.py

A Python Toolbox for Statistics and Neurophysiological Signal Processing (EEG, EDA, ECG, EMG...).
http://neurokit.rtfd.io
MIT License
366 stars 102 forks source link

Use non-standard filter for ECG #14

Closed stjep closed 7 years ago

stjep commented 7 years ago

This may very well be a silly question because I haven't looked through your thoroughly enough (or because I'm missing something painfully obvious), so please bear with me.

From what I have been able to figure out by quickly scanning your code, you're using biosppy's filtering routine to filter ECG data prior to HRV/etc extraction. The biosppy filter, in turn, is set to a default bandpass filter at [3, 45] Hz.

If I wanted to run NeuroKit's ECG routine (processed_ecg = nk.ecg_process(ecg_signal, resp_signal)), but specify a different filter (say, just a high pass filter?), is there an easy way to do this? I figured that if all else fails I can edit ecg.py within biosppy…

DominiqueMakowski commented 7 years ago

That's actually a pretty good question. As I'm not an expert in filtering and stuff, the suggestions I'm about to make might be not relevant. However, I see a quick/messy way and a neat way of doing that. For the messy fix, maybe you could apply the desired filter (if it's more stringent than biosppy's one) after, or before running neurokit's routines?

As for the "clean" ways, these might be: 1) to change biosppy's function so that the filter parameters are arguments, and then add these parameters to neurokit. 2) Within ecg_process, I use at the beginning biosppy.ecg.ecg. So in order to avoid modifying (and eventually messing) with biosppy, maybe we could decompose my use of it, by shamelessy copying parts of the inside of biosppy's function 😅 so that the filter prameters are apparent and controlable.

What do you think about these suggestions? Have you identified, within biosppy, where the filtering with the default parameters happens?

stjep commented 7 years ago

As far as I can tell, biosppy implements it's filtering here: https://github.com/PIA-Group/BioSPPy/blob/master/biosppy/signals/ecg.py#L71

The parameters that are supported are listed here: https://github.com/PIA-Group/BioSPPy/blob/master/biosppy/signals/tools.py#L247

So, perhaps one way to achieve what I'm trying to do is to set biosppy_ecg["filtered"] within neurokit's routine to be equal to biosppy.tools.filter_signal with whatever parameters I want to try.

In my case I wanted to try a highpass filter at 0.5 Hz, which is a little less conservative than the 3 Hz cut-off that biosppy defaults to. (On an unrelated note, I would probably stick with the default if biosppy detailed why they use that setting.)

DominiqueMakowski commented 7 years ago

Would you like do give it a try by attemtping at a first commit :)? I think neurokit would profit from more flexibility and control. Btw, if you find some references or reasons about the good default parameters, let me know, we could include them in the documentation.

stjep commented 7 years ago

I'll give it a try and will let you know if I get nowhere.

I'll also reach out to the biosppy devs. It'd be good to know why they chose those settings, and what the implications are if you're not using a single-channel Lead I setup

capcarr commented 7 years ago

Hi, I'm BioSPPy's maintainer

The default parameters for ECG filtering were chosen because we mainly acquire ECG from the hands, without using any gel or adhesive. Therefore, the signal is significantly more noisy than clinical-grade acquisitions, in particular regarding baseline wander, thus the use of the 3 Hz cut-off. This was a compromise between effectively removing the noise and keeping the signal's information, which we determined experimentally.

Regarding having additional parameters to biosppy.signals.ecg.ecg, I would argue against it, given the purpose of that function (and the corresponding functions in the other signals) is to be as simple as possible and consistent across signal types. I would suggest "deconstructing" the function in your own code.

DominiqueMakowski commented 7 years ago

@capcarr Thanks for your precisions! @stjep deconstruction is the right way to go then :) I've created a new branch called ecg_filtering to which we can safely commit the changes. Once it's working, we'll merge it to master.

DominiqueMakowski commented 7 years ago

@stjep I've found some free time to make some progress on this feature 😸 . I created the ecg_preprocess function that desconstructs the high-level biosppy.ecg.ecg, and then pulled out the filtering parameters up to ecg_process and bio_process. Could you run some tests to see if it works with other filter parameters? Thanks and let know how it goes!

stjep commented 7 years ago

@DominiqueMakowski, I started tinkering on Friday, but didn't get very far (and what I was thinking isn't as elegant as your code).

I'm going to play around with it today and tomorrow and will report back then.

I'll also add to the code some pointers on where to go to find out how these filters work. I no expertise when it comes to filter design, but it is something that a lot of people care about, and there are a lot of hidden assumptions in biosppy's implementation (i.e., they don't expose the windowing and order parameters that you can set for these filters).

If you're planning to document this function somewhere else let me know and I can throw that info in there too.

DominiqueMakowski commented 7 years ago

@stjep We could actually further open the preprocessing function to expose, like you said, the windowing and order parameters. Also, maybe allow the user to skip some steps like filtering or so (I'm not sure it makes sense though)? I like the idea of having both good and justified defaults and, at the same time, the possibility of having them modified.

As for the documentation, there are two things:

stjep commented 7 years ago

Also, maybe allow the user to skip some steps like filtering or so

A lot of data (maybe most?) will be collected with hardware filters applied as the data is collected. This data could be clean enough to allow for analysis without further digital filtering. My understanding is that, generally, you want to keep filtering to a needs basis because filters change the data (this is based on what little I know about EEG). So, it would allow for greater flexibility for users to be able to pick the default, specify the filter, or skip the filtering step entirely.

I'll expand the docs and tutorials some time this week 😀