[ENH] Add forward IIR filtering

[qbarthelemy]

What does this implement/fix?

Real-time EEG based systems like BCI or neurofeedback use only causal filters, specially forward IIR filters to avoid delay. Replaying data in offline mode, we want to simulate the same processing applied in real-time. However, forward IIR filters are not currently available in MNE.

This PR uses parameters method='iir' and phase='minimum' of mne.filter.filter_data to allow forward IIR filtering, using scipy.signal.lfilter or scipy.signal.sosfilt according to iir_params.

[welcome[bot]]

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

[wmvanvliet]

CI complains because there are still a reference to the old _filtfilt function in mne/viz/_figure.py

[larsoner]

Looks reasonable to me, can you add an entry to doc/changes/latest.inc with a new entry in doc/changes/names.inc?

It would be nice to have some test that phase='zero' produces a different output from that of phase='minimum', too. Even something as simple as generating a short segment of randn data and ensuring that the power_orig > power_minimum > power_zero would be good enough in this case probably. Or maybe better, put a delta (a single 1.) in the middle of a bunch of zeros and ensure in the phase='zero' case the np.argmax is still equal to the middle index, and that in the 'minimum' case it has shifted rightward.

[cbrnr]

It would be great if you could also update the filtering tutorial with the new IIR minimum phase option.

[cbrnr]

Thanks @qbarthelemy for updating the tutorial. However, looking at the new figure of the causal IIR filter (https://output.circle-artifacts.com/output/job/d5a26819-ebf0-4d38-8690-5c99057b5be3/artifacts/0/dev/auto_tutorials/preprocessing/25_background_filtering.html#designing-iir-filters) begs the question why anyone would prefer a causal IIR to a minimum-phase FIR filter?

There's even a paragraph stating:

Note that this minimum-phase filter has a much smaller delay compared to an uncompensated, causal linear-phase filter, as shown below. This is why in MNE-Python we currently only offer zero-phase, non-causal filters (i.e., delay-compensated filters) or minimum-phase causal filters.

If we add causal IIR filters, we should update this paragraph as well, but again, currently I do not see a good reason to use such filters.

I'm also not sure that we should use phase='minimum' for such IIR filters. Are these filters really minimum phase (i.e. zeros inside the unit circle)? At least for FIR filters, minimum-phase is just one option for causal filters, because there are also linear phase FIR filters.

[qbarthelemy]

why anyone would prefer a causal IIR to a minimum-phase FIR filter?

Real-time applications like brain-computer interfaces or neurofeedback use forward IIR filters to have small delay. Replaying data in offline mode with MNE, we want to simulate the same processing applied in real-time. @cbrnr

I'm also not sure that we should use phase='minimum' for such IIR filters.

I re-use the already existing option phase='minimum' because it is the choice that has the least impact on the code.

[cbrnr]

Real-time applications like brain-computer interfaces or neurofeedback use forward IIR filters to have small delay. Replaying data in offline mode with MNE, we want to simulate the same processing applied in real-time.

So no good reason, just that some people use it, right? I just want to understand why they would not use a minimum-phase FIR filter. Do you know?

I re-use the already existing option phase='minimum' because it is the choice that has the least impact on the code.

But I think this is technically incorrect. Unless you can show that the phase is indeed minimum, I'd prefer a different term (not sure which one, maybe someone has a good idea).

[qbarthelemy]

So no good reason, just that some people use it, right? I just want to understand why they would not use a minimum-phase FIR filter. Do you know?

For the same kind of behavior (roll-of, band stop attenuation, band pass ripple, insertion loss), order of IIR filter will be lower than FIR counterpart (even minimum-phase). Consequently, this IIR filter will have a lower latency (delay between input sample and its filtered version), that is a crucial property in real-time applications.

But I think this is technically incorrect. Unless you can show that the phase is indeed minimum, I'd prefer a different term (not sure which one, maybe someone has a good idea).

Ok. Waiting for the appropriate term.

[cbrnr]

Compare the FIR minimum-phase with the causal IIR filter in the tutorial:

The minimum-phase FIR has a much smaller delay in the passband (slightly above zero), whereas the causal IIR is always slower and even has some peaks with much higher delay in the passband. In addition, passband ripple and roll-off are much worse for the IIR than for the minimum-phase FIR.

I'm not saying we should not add causal IIR, but I would like to understand why someone should use them.

[qbarthelemy]

Sounds like a misunderstanding about delay word : I speak about filter latency (the delay between an input sample and its filtered output version), not about group or phase delay.

[cbrnr]

Those are the same thing. The lower panels in the figures show the delay in seconds.

[larsoner]

I think perhaps @qbarthelemy means computational delay/overhead from the CPU time it takes to actually perform the filtering step. IIR has fewer coefficients hence less computational overhead. This application is where I see IIR used most often/justifiably at least, so it seems okay to add it for completeness for BCI pipeline comparisons.

For the name I like phase='forward' because phase='zero' for IIR is typically called a forward-backward filter, and your new option allows running just the forward pass. This new 'forward' should not be allowed for FIR, and 'minimum' should not be allowed for IIR.

[cbrnr]

I think perhaps @qbarthelemy means computational delay/overhead from the CPU time it takes to actually perform the filtering step. IIR has fewer coefficients hence less computational overhead. This application is where I see IIR used most often/justifiably at least, so it seems okay to add it for completeness for BCI pipeline comparisons.

I don't think this is relevant for BCIs, at least it wasn't when I was still working on online BCIs. Maybe if you want to implement it on some embedded system, but certainly not on any half-decent PC.

Interestingly, we also used IIR filters because we wanted to achieve short delays – I have no idea why nobody suggested minimum-phase FIR filters.

So the only reason why IIR filters are needed is to reproduce someone else's pipeline, but technically I have yet to hear a real argument. I'm trying to align this addition with our tutorial, which should clearly mention why we added causal IIR filtering and when you should choose this variant over e.g. minimum-phase FIR.

For the name I like phase='forward'

:+1:

[larsoner]

I don't think this is relevant for BCIs, at least it wasn't when I was still working on online BCIs.

It sounds like people do still use use it, which makes it relevant. If we implement it here it can immediately be used in mne-realtime for example for comparison studies.

So the only reason why IIR filters are needed is to reproduce someone else's pipeline, but technically I have yet to hear a real argument.

1. To me the fact that people have used these a lot previously makes it potentially worthwhile, even if minimum phase is probably a better choice in most circumstances.
2. Along those lines, there is collective community knowledge here. For example, if I say I did a foward 2-pole Butterworth lowpass at 40, 90% of people with signals expertise will know what I mean. If I say I used a minimum-phase FIR filter with 40 taps, you've already lost a lot of people -- and I haven't even specified the filter completely yet. I have collaborators who regularly use forward-only IIR because it's easy and well established in this way. The fact that you did this in the past also speaks to a community acceptance/use level that I think warrants inclusion.
3. Even though you don't think the performance is/should be relevant here, it could be a motivating factor for some people. Maybe they shouldn't use Python, but we might as well make things faster when we can.
4. I reviewed a paper recently where they used a forward IIR in a realtime OPM processing scheme, and I think it was the right choice -- at least a totally reasonable one -- for their use case. If we had this in MNE-Python, they maybe could have just done filter_data(...) instead of messing with SciPy, and then easily tested the minimum_phase as well while they did it. (So it's not just useful for BCI use cases.)

My vote is to implement it -- it's easy to do and support -- and add it to the tutorial and show that it's maybe not as good as the minimum phase FIR, but say that computationally the cost is lower and it has been used in the literature.

[cbrnr]

I agree @larsoner, these are all good points, and I was never arguing for not including it in MNE. However, we should make a good job of explaining why IIR forward filters are needed (useful?), and currently this is not the case. If you look at the two figures, who would choose the IIR filter? But again, if you think that lower computational costs is sufficient, then let's include that and maybe some very carefully worded cautionary warning.

[larsoner]

@qbarthelemy do you want try changing the phase parameter value string and updating the tutorial? You also need a rebase or merge with main. Let me know if you want help with any of those and I can push commits

[larsoner]

FYI @qbarthelemy we plan to release 1.2 next week. If you can get this in by then, great, if not then it's fine to get it into 1.3 instead!

[qbarthelemy]

Sorry @larsoner for not responding to your repeated reminders, but I was busy on other projects.

Thank you for your open-minded point of view. I daily use single coeff IIR filters for real-time processing, allowing to filter a lot more streams than nice but lengthy minimum-phase filters.

With my friends, we were not able to retrieve, with offline replay, results obtained during online BCI, but our article has been published, without online results, unfortunately for the BCI community which struggles to close the gap between online and offline performances.

I see that forward_iir branch has already been forked, consequently I have commited my last local changes (updating log and changing phase into 'forward').

[welcome[bot]]

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪