Strange filter order expected by ecg_preprocess

[renatosc]

I was having a hard time to have nk.ecg_preprocess to filter my signal data using a "butter" filter of order 4. Looking the source code I see that the function is multiplying the order parameter by the sampling rate:
order = int(filter_order * sampling_rate)

So, after I passed the order parameter as 4/250 (250 was my signal sampling rate) I got it to work as expected.

Is there any reason why that function is expecting the filter order paramenter to be divided by the sampling rate? AFAIK, that is not normal behavior expected by the biosppy.signals.tools.filter_signal function (which nk.ecg_preprocess uses) or any other filtering libraries I have used so far (SciPy, Matlab,...).

[DominiqueMakowski]

I guess this part was based on biosppy, that multiples 0.3 by the sampling rate internally (hence the 0.3 as a default order).

However, this might be correct with only particular filters (guess I never thought about this as I never had the occasion of using different filters). Maybe we could allow the order parameter to be something like order="default", which would then be set appropriately (to some standard values depending on the filter type)?

What do you think?

[renatosc]

Sorry for taking too long to reply. I looked the biosppy code and it appears to me that they added that 0.3 multiplication line just as simplification to internally calculate a order based on the sampling rate provided since the order is not a parameter itself. In our case, the ecg_preprocess accepts order as a parameter (filter_order) and have it multiplied by the sampling rate does not appear correct to me. But I am no expert in DSP and also a change in that point could break code of users. What do you think? Leave that way?

[DominiqueMakowski]

Hum, the idea was just to expose the order parameter (further adjusted based on the sampling rate). Instead of being hard-coded to 0.3 as biosppy, it is set as default to 0.3 but can be changed if need be. To be honest, I never had to change this parameter, but I guess that for some other filter types it could make sense to change it.
Do you know how we could change it for the better?

[renatosc]

The problem is not the default value, but what you do with the value after:
order = int(filter_order * sampling_rate)

Because right now if I enter, let's say, filter_order = 4, the code will actually use 4 * sampling rate for the order, which is not intuitive from the outside.

[DominiqueMakowski]

Right, would you suggest then improving the documentation (specifying that the order value is further multiplied by the sampling rate), or changing the way it is handled internally?

[renatosc]

That is actually the question that I asked you...hehe. The old dilema of making a breaking change or not... I think for now the documentation note would be enough. That way we don't create a incompatible update without previous notification.