circular convolution implementation

[a2ray]

Hi folks,

In the implementation of circular shift here:

SignalAnalysis.jl/src/dsp.jl

Line 332 in a9a968e

function circconv(x::AbstractVector, y::AbstractVector=x)

function circconv(x::AbstractVector, y::AbstractVector=x)
  if length(x) != length(y)
    throw(ArgumentError("x and y must be of equal length"))
  end
  n = length(x)
  z = similar(x)
  for j ∈ 1:n
    z[j] = circshift(x, j-1)' * y
  end
  return z
end

I believe it should be instead

function circconv(x::AbstractVector, y::AbstractVector=x)
  if length(x) != length(y)
    throw(ArgumentError("x and y must be of equal length"))
  end
  n = length(x)
  z = similar(x)
  xuse = circshift(reverse(x), 1) # CHANGED
  for j ∈ 1:n
    z[j] = circshift(xuse, j-1)' * y # CHANGED
  end
  return z
end

I made the same mistake when trying to figure out why a convolution behaved the way it did (instead of doing circular convolution using FFT multiplication)

[mchitre]

#45 fixes this. @a2ray would you like to take a quick look the verify before I merge?

[a2ray]

circconv looks good to me and circcorr follows the definition. I suppose for circcorr to be linear as opposed to circular it also needs both sequences of length n and m to be zero padded to n+m-1? Lastly, another test could be added interchanging input sequences to ensure that the result is commutative.