Back to the main page.

Bug 2766 - fir_filterdcpadded speed improvement suggestion

Reported 2014-11-21 14:58:00 +0100
Modified 2015-01-12 09:18:27 +0100
Product: FieldTrip
Component: preproc
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P5 normal
Assigned to: Jan-Mathijs Schoffelen
Depends on:
See also:

Jan-Mathijs Schoffelen - 2014-11-21 14:58:23 +0100

When using a firws filter with a large filter order (in my case >10000) on long segments of data fir_filterdcpadded gets extremely (prohibitively) slow due to the use of the filter.m function. An obvious speed improvement can be achieved by using fftfilt instead of filter. @Andreas: do you see any objection in building in a switch that is either using fftfilt or filter based on some heuristic?

Jan-Mathijs Schoffelen - 2014-11-21 15:08:51 +0100

adding to my first comment: if agreed, I will build it in, Andreas doesn't have to do it.

- 2014-11-25 17:39:30 +0100

Created attachment 677 simple switch for

- 2014-11-25 17:39:42 +0100

Define "prohibitively" ;) I would have no objections against building in fftfilt support. However, I would like to avoid changing default behavior for two reasons: filter and fftfilter do not give numerically identical results. This would compromise backward compatibility and replicability, e.g. for re-analysis or comparison to other software packages. And, less critical, with some initial testing I got some quite different results for the critical filter length on different platforms. Not sure whether this is due to OS, or CPU, or library optimization differences. And not sure whether there is really a simple heuristic. I would suggest simply using a flag as in the attached example (with a default of 0). If you want to implement a different more complex solution I'm open but I would like to ask for permission to port this solution also to EEGLAB in order to keep the low-level functions in sync. Best, Andreas

Jörn M. Horschig - 2014-11-26 08:49:24 +0100

Hi Andreas, I am pretty sure you do not need to ask permission from us to do anything ;) Best, Jörn

Jan-Mathijs Schoffelen - 2014-11-27 13:09:01 +0100

(In reply to widmann from comment #3) My heuristic would be: switch to fftfilt if it takes longer to filter a single trial than for me to fetch a coffee. (to give an indication: my office is on the second floor, and due to the fact that the building is being renovated at the moment, I need to take the far end staircase, which results in me having to traverse a long corridor twice, because the coffee room is essentially right below my office, but two floors down)....... Thanks for the patch!

- 2014-11-27 14:55:19 +0100

Created attachment 679 usefftfilt patch

- 2014-11-27 14:57:04 +0100

So for the sake of your health a complete patch attached ;) I hope I didn't miss anything obvious. Do you integrate the patch and close the bug? Best, Andreas

Jan-Mathijs Schoffelen - 2014-11-27 15:04:55 +0100

I will incorporate it soon and close. Thanks, Andreas.

Jan-Mathijs Schoffelen - 2014-12-01 08:59:22 +0100

Hi Andreas, Apologies, but I did not notice that you contributed a full git-based patch. I am still using svn, so I will need some directions how to incorporate your patch.

- 2014-12-01 12:39:35 +0100

Created attachment 680 usefftfilt patch v2

- 2014-12-01 12:44:50 +0100

This is a regular patch you should be able to apply with patch -p0 < fftfilt4fieldtrip_v2.patch if fftfilt4fieldtrip_v2.patch is in the root folder of your fieldtrip installation. I just noticed that I forgot to add the documentation for the new usefftfilt option in private/preproc.m. Please use the new v2 version of the patch. Best, Andreas

Jan-Mathijs Schoffelen - 2014-12-01 16:41:17 +0100

bash-4.1$ patch -p0 < fftfilt4fieldtrip_v2.patch patching file ft_preprocessing.m patching file preproc/ft_preproc_bandpassfilter.m patching file preproc/ft_preproc_bandstopfilter.m patching file preproc/ft_preproc_highpassfilter.m patching file preproc/ft_preproc_lowpassfilter.m patching file preproc/private/filter_with_correction.m patching file preproc/private/fir_filterdcpadded.m patching file private/preproc.m bash-4.1$ svn commit -m "enhancement - added optional fftfilt for firws filter" ft_preprocessing.m preproc/ft_preproc_*filter.m private/preproc.m Sending ft_preprocessing.m Sending preproc/ft_preproc_bandpassfilter.m Sending preproc/ft_preproc_bandstopfilter.m Sending preproc/ft_preproc_highpassfilter.m Sending preproc/ft_preproc_lowpassfilter.m Sending private/preproc.m Transmitting file data ...... Committed revision 9989. Thanks, Andreas!

- 2014-12-01 16:46:10 +0100

You did not commit the low-level functions! preproc/private/filter_with_correction.m preproc/private/fir_filterdcpadded.m Best, Andreas

Jan-Mathijs Schoffelen - 2014-12-01 17:03:47 +0100

oopsie. Just did it. Sorry about that. I could also just say that it was on purpose, because we are nearing the 10000'th commit in svn, and it would be cool to claim it. (now at #9991)...