Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve decision boundary between time domain and frequency filtering #273

Merged
merged 6 commits into from
May 3, 2019

Conversation

galenlynch
Copy link
Member

In my testing, filt is choosing to use frequency domain filtering in instances when time domain filtering would be faster. I have moved the decision boundary between the two algorithms to be better reflect benchmarking results from my laptop. To be honest, I'm not sure how well these benchmarks generalize.

In the process, I have also changed how nfft is chosen for the overlap-save algorithm. The new heuristic is simpler, and seems to follow the cost estimates provided by FFTW.

The choice between time and frequency domain filtering is still not perfect. Right now the decision seems to be correct when filtering large amounts of data. However, for smaller amounts of data, it may be advantageous to do time domain filtering to avoid the overhead of frequency domain filtering, even if it would be more expensive per output sample for large amounts of data.

Benchmarking reveals that time domain filtering is faster than frequency domain
filtering if the unrolled-loop functions are used.
In my testing it is faster to use time domain filtering for filter sizes of
length 54 or smaller.
@galenlynch galenlynch changed the title Filt choose alg improvement Improve decision boundary between time domain and frequency filtering Apr 2, 2019
galenlynch added a commit to galenlynch/DSP.jl that referenced this pull request Apr 19, 2019
@galenlynch
Copy link
Member Author

Ok to merge?

@galenlynch galenlynch merged commit e004b80 into JuliaDSP:master May 3, 2019
@galenlynch galenlynch deleted the filt_choose_alg_improvement branch May 3, 2019 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant