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

Use non-standard filter for ECG #14

Closed
stjep opened this issue Jul 6, 2017 · 10 comments
Closed

Use non-standard filter for ECG #14

stjep opened this issue Jul 6, 2017 · 10 comments

Comments

@stjep
Copy link

stjep commented Jul 6, 2017

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
Copy link
Member

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
Copy link
Author

stjep commented Jul 6, 2017

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
Copy link
Member

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
Copy link
Author

stjep commented Jul 6, 2017

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
Copy link

capcarr commented Jul 7, 2017

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
Copy link
Member

@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
Copy link
Member

@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
Copy link
Author

stjep commented Jul 10, 2017

@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
Copy link
Member

@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:

  • The functions documentation. In fact, when you write within the string section just below the function (the docstrings), it will automatically be transformed and uploaded in the API documentation. So do not hesitate to complete these docs (either under the parameters section or within the "details", as I started to do with ecg_hrv)
  • The tutorials. As you've seen with the small tutorial for biosignal processing, I'd like to expand it by also having some theorethical parts about the signals, their physiological meaning, the processing, the acquisition and such. The documentation lies here, within the tutorial folder. If you have any ideas or things that we could write, again feel free to contribute :)

@stjep
Copy link
Author

stjep commented Jul 10, 2017

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 😀

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

No branches or pull requests

3 participants