Back to the main page.
Bug 2766 - fir_filterdcpadded speed improvement suggestion
| Status | CLOSED FIXED |
| 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 |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| 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