search

JuliaDSP / DSP.jl

Filter design, periodograms, window functions, and other digital signal processing functionality
https://docs.juliadsp.org/dev/
Other
374 stars 108 forks source link

Change documentation for xcorr and conv to be true to the actual implementation. #521

Closed ErikBuer closed 6 months ago

ErikBuer commented 6 months ago

It looks like there was an intention to implement a clever selection between time domain and frequency domain convolution, but this was never implemented.

The documentation currently says that one of the two domain methods are used, where in reality only FFT convolution is implemented. As it is about 5 years since the functions was last changed, I feel it is time to let go, and just describe what the function does currencty.

If you have any thoughts, I'd love to hear them :)

wheeheee commented 6 months ago

Doesn't unsafe_conv_kern_os! implement overlap-save?

ErikBuer commented 6 months ago

Hmm. I may have mixed this up a bit.

Yes it appears it does "overlap-save?" in ´unsafe_conv_kern_os!´.

I mistok the overlap-save for a time-domain convolution, that was listed as "TODO" in _conv!

# Does convolution, will not switch argument order
function _conv!(out, u, v, su, sv, outsize)
    # TODO: Add spatial / time domain algorithm
    _conv_fft!(out, u, v, su, sv, outsize)
end

Thank you for the correction @wheeheee :)

wheeheee commented 6 months ago

Searched it up, this probably refers to #292