sfstoolbox / sfs-matlab

SFS Toolbox for Matlab/Octave
https://sfs-matlab.readthedocs.io
MIT License
97 stars 39 forks source link

Avoid `symmetric` parameter in `ifft()` #133

Closed hagenw closed 7 years ago

hagenw commented 7 years ago

The symmetric option is used currently in interpolate_ir(), compare also #130, and modal_weighting(). Under Octave this option is not available for ifft(), which means we have to replace it.

One solution might be to replace it by real(ifft(spec)); as it is done in signal_from_spectrum(). I guess one problem might be that this will be not as fast as the other option.

@chris-hld could you please test the performance of interpolate_ir() with the old symmetric option and the real(ifft()) version.

chris-hld commented 7 years ago

For general signals, the performance is surprisingly similar and fast. For the example I tested (4 channels of a few seconds input), I could not reliably tell, which one is faster. I am still a bit worried about the output though, since it can be quite different. As long as everything is correct there shouldn't be any imaginary part in the first place. 'symmetric' restores and corrects the input, while real() just discards everything.

For test_interpolation_methods(1) 'symmetric' : 0.9292 s real() : 0.9209 s on Xeon CPU @ 3.50GHz

chris-hld commented 7 years ago

There is a bug somewhere in interpolate_ir()! The remaining imaginary part of ir is around 0.3 -- this is definitely not round-off errors. I'll have a deeper look at it, any ideas?

chris-hld commented 7 years ago

https://github.com/sfstoolbox/sfs-matlab/pull/153

hagenw commented 7 years ago

Just for testing, I implemented a version with an extra function that checks for Octave and uses than real(ifft(...)). Under Matlab still ifft(...,'symmetric') is used: #154

chris-hld commented 7 years ago

https://github.com/sfstoolbox/sfs-matlab/pull/155

hagenw commented 7 years ago

Solved by #155