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

Got "BoundsError" while trying to use resample function. #317

Closed
Losses opened this issue Sep 11, 2019 · 8 comments · Fixed by #539
Closed

Got "BoundsError" while trying to use resample function. #317

Losses opened this issue Sep 11, 2019 · 8 comments · Fixed by #539
Labels

Comments

@Losses
Copy link

Losses commented Sep 11, 2019

using DSP

a_sig = sin.([1:1:35546; ])
resample(a_sig, 1/55.55)

And we will get:

ERROR: BoundsError: attempt to access 640-element Array{Float64,1} at index [641]
Stacktrace:
 [1] setindex! at .\array.jl:767 [inlined]
 [2] filt!(::Array{Float64,1}, ::FIRFilter{DSP.Filters.FIRArbitrary{Float64}}, ::Array{Float64,1}) at C:\Users\Losses\.juliapro\JuliaPro_v1.1.1.1\packages\DSP\wwKNu\src\Filters\stream_filt.jl:660
 [3] filt(::FIRFilter{DSP.Filters.FIRArbitrary{Float64}}, ::Array{Float64,1}) at C:\Users\Losses\.juliapro\JuliaPro_v1.1.1.1\packages\DSP\wwKNu\src\Filters\stream_filt.jl:673
 [4] resample(::Array{Float64,1}, ::Float64, ::Array{Float64,1}) at C:\Users\Losses\.juliapro\JuliaPro_v1.1.1.1\packages\DSP\wwKNu\src\Filters\stream_filt.jl:728
 [5] resample(::Array{Float64,1}, ::Float64) at C:\Users\Losses\.juliapro\JuliaPro_v1.1.1.1\packages\DSP\wwKNu\src\Filters\stream_filt.jl:733
 [6] top-level scope at none:0
@galenlynch galenlynch added the bug label Oct 1, 2019
@galenlynch
Copy link
Member

@JayKickliter It looks like you wrote this code originally. I'd have to spend a bit of time reading it to understand why the indexing logic doesn't work in this instance. Do you have time to take a look at it?

@JayKickliter
Copy link
Member

My first guess is that it’s floating point rounding problem, but I’ll take a closer look. I haven’t written a single line of Julia in several years, so it’ll take me little time to get back up to speed.

@galenlynch
Copy link
Member

Thanks! I'll also take a look.

@galenlynch
Copy link
Member

#262 is the same bug.

@mchitre
Copy link

mchitre commented Mar 19, 2022

Another MWE, if it helps:

using DSP
x = randn(1822)
resample(x, 0.9802414928649834) # works
resample(x, 0.9802414928649835) # fails

@minecraft2048
Copy link

I'm also hitting this problem on Julia 1.9 beta and DSP v0.7.8:

julia> using DSP
julia> resample(1:16_367_000*2, 10_000_000/16_367_000)
ERROR: BoundsError: attempt to access 20000000-element Vector{Float64} at index [20000001]
Stacktrace:
 [1] setindex!
   @ ./array.jl:969 [inlined]
 [2] filt!(buffer::Vector{Float64}, self::FIRFilter{DSP.Filters.FIRArbitrary{Float64}}, x::Vector{Int64})
   @ DSP.Filters ~/.julia/packages/DSP/wENjI/src/Filters/stream_filt.jl:660
 [3] filt(self::FIRFilter{DSP.Filters.FIRArbitrary{Float64}}, x::Vector{Int64})
   @ DSP.Filters ~/.julia/packages/DSP/wENjI/src/Filters/stream_filt.jl:673
 [4] resample(x::UnitRange{Int64}, rate::Float64, h::Vector{Float64})
   @ DSP.Filters ~/.julia/packages/DSP/wENjI/src/Filters/stream_filt.jl:728
 [5] resample(x::UnitRange{Int64}, rate::Float64)
   @ DSP.Filters ~/.julia/packages/DSP/wENjI/src/Filters/stream_filt.jl:733
 [6] top-level scope
   @ REPL[6]:1

@anowacki
Copy link
Contributor

I have hit this multiple times now. My own reproducer is

import DSP
DSP.resample(zeros(1000), 0.012)

It does appear that somewhere with the FIRArbitrary constructor or its filt! method there is some sort of rounding problem which means that kernel.xIdx gets to the length of the input vector after one point too many (making the buffer index go out of bounds). I don't believe the problem is with outputlength(::FIRArbitrary, inputlength), as this gives the same value as ceil(Int, inputlength*rate) for all the problem cases above. That leaves the issue potentially in update(::FIRArbitrary) (why not update!, btw?).

Given that filt(::FIRFilter{FIRArbitrary}, x) (src/Filters/stream_filt.jl:670) creates a buffer and then maybe shrinks it anyway, perhaps it makes sense to create a buffer there which is 1 element longer? The downside here is that in almost all cases the array will be shrunk by one. But given that there are lots of allocations happening in a call to filt anyway and resize! can be cheap, this probably doesn't matter.

For what it's worth, this version of filt(::FITFilter{FIRArbitrary}}, x) works for all the failing examples above:

function filt(self::FIRFilter{FIRArbitrary{Th}}, x::AbstractVector{Tx}) where {Th,Tx}
    bufLen         = DSP.outputlength(self, length(x)) + 1 # Note extra element here
    buffer         = Vector{promote_type(Th,Tx)}(undef, bufLen)
    samplesWritten = filt!(buffer, self, x)

    samplesWritten == bufLen || resize!(buffer, samplesWritten)

    return buffer
end

@JayKickliter
Copy link
Member

@anowacki It's been years since I wrote this code, and I'd do a lot differently now. I also barely remember any Julia, let alone anything post 0.5. I do think your findings are correct and suggest opening a PR.

anowacki added a commit to anowacki/DSP.jl that referenced this issue Feb 18, 2024
In some cases when `filt(::FIRFilter{FIRArbitrary}, x)` is called
with certain values of `x`, the buffer is actually one sample too
short and a `BoundsError` is thrown in
`filt!(buffer, ::FIRFilter{FIRArbitrary}, x)`.  Add one extra sample
to the calculated buffer length to catch these exceptional cases, which
mostly arise when resampling with particular resampling ratios.

(This code is really a hack and simply works around the deeper problem
of calculating the correct output buffer length; this should instead be
addressed properly in a future change.)

Closes JuliaDSP#317.
anowacki added a commit to anowacki/DSP.jl that referenced this issue Feb 18, 2024
In some cases when `filt(::FIRFilter{FIRArbitrary}, x)` is called
with certain values of `x`, the buffer is actually one sample too
short and a `BoundsError` is thrown in
`filt!(buffer, ::FIRFilter{FIRArbitrary}, x)`.  Add one extra sample
to the calculated buffer length to catch these exceptional cases, which
mostly arise when resampling with particular resampling ratios.

(This code is really a hack and simply works around the deeper problem
of calculating the correct output buffer length; this should instead be
addressed properly in a future change.)

Closes JuliaDSP#317.
anowacki added a commit to anowacki/DSP.jl that referenced this issue Feb 20, 2024
In some cases when `filt(::FIRFilter{FIRArbitrary}, x)` is called
with certain values of `x`, `filt!(buffer, ::FIRFilter{FIRArbitrary}, x)`
tries to write one sample too many to the buffer and a `BoundsError`
is thrown.  Add one extra sample to the buffer to catch these
exceptional cases.

(This code is really a hack and simply works around the deeper problem
of calculating the correct output buffer length; this should instead be
addressed properly in a future change.)

Closes JuliaDSP#317.
martinholters pushed a commit that referenced this issue Mar 11, 2024
In some cases when `filt(::FIRFilter{FIRArbitrary}, x)` is called
with certain values of `x`, `filt!(buffer, ::FIRFilter{FIRArbitrary}, x)`
tries to write one sample too many to the buffer and a `BoundsError`
is thrown.  Add one extra sample to the buffer to catch these
exceptional cases.

(This code is really a hack and simply works around the deeper problem
of calculating the correct output buffer length; this should instead be
addressed properly in a future change.)

Closes #317.
martinholters pushed a commit that referenced this issue Sep 13, 2024
In some cases when `filt(::FIRFilter{FIRArbitrary}, x)` is called
with certain values of `x`, `filt!(buffer, ::FIRFilter{FIRArbitrary}, x)`
tries to write one sample too many to the buffer and a `BoundsError`
is thrown.  Add one extra sample to the buffer to catch these
exceptional cases.

(This code is really a hack and simply works around the deeper problem
of calculating the correct output buffer length; this should instead be
addressed properly in a future change.)

Closes #317.

(cherry picked from commit 73d3214)
martinholters pushed a commit that referenced this issue Sep 13, 2024
In some cases when `filt(::FIRFilter{FIRArbitrary}, x)` is called
with certain values of `x`, `filt!(buffer, ::FIRFilter{FIRArbitrary}, x)`
tries to write one sample too many to the buffer and a `BoundsError`
is thrown.  Add one extra sample to the buffer to catch these
exceptional cases.

(This code is really a hack and simply works around the deeper problem
of calculating the correct output buffer length; this should instead be
addressed properly in a future change.)

Closes #317.

(cherry picked from commit 73d3214)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants