Skip to content

1D Convolution in Julia - convolve_cyclic not commutative #853

Closed
@jiegillet

Description

@jiegillet

Bug Report

Description

The convolution operation should be commutative, but the Julia implementation isn't for inputs of different size.

Steps to Reproduce

julia> convolve_cyclic([3, 4, 5], [1, 2])
3-element Array{Float64,1}:
 13.0
 13.0
 10.0

julia> convolve_cyclic([1, 2], [3, 4, 5])
ERROR: BoundsError: attempt to access 2-element Array{Int64,1} at index [3]
Stacktrace:
 [1] getindex at ./array.jl:809 [inlined]
 [2] convolve_cyclic(::Array{Int64,1}, ::Array{Int64,1}) at /Users/jie/Documents/algorithm-archive/contents/convolutions/code/julia/1d_convolution.jl:17
 [3] top-level scope at REPL[52]:1

When the signal is larger, the filter is padded with 0s (conceptually, maybe not in the implementation). It should go the other way around too. The following lines even suggest that:

# output size will be the size of sign
output_size = max(length(signal), length(filter))

Expected behavior

julia> convolve_cyclic([3, 4, 5], [1, 2])
3-element Array{Float64,1}:
 13.0
 13.0
 10.0

julia> convolve_cyclic([1, 2], [3, 4, 5])
 13.0
 13.0
 10.0

For Algorithm Archive Developers

  • The bug can be reproduced
  • The bug can be fixed (if not, please explain why not in a comment below)
  • There is a timeline to fix the bug
  • The bug has been fixed (Please link the PR)

Metadata

Metadata

Assignees

No one assigned

    Labels

    ProblemThis is a problem in the archive or an implementation.lang: juliaJulia programming language

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions