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

Radix sort is broken for OffsetArrays with negative offset #56

Closed
LilithHafner opened this issue Jun 6, 2022 · 5 comments · Fixed by #63
Closed

Radix sort is broken for OffsetArrays with negative offset #56

LilithHafner opened this issue Jun 6, 2022 · 5 comments · Fixed by #63

Comments

@LilithHafner
Copy link
Member

using SortingAlgorithms, OffsetArrays
sort!(OffsetArray(rand(1000), -500);  alg=RadixSort)

With julia --check-bounds=yes, I get

ERROR: BoundsError: attempt to access 1000-element OffsetArray(::Vector{Float64}, -499:500) with eltype Float64 with indices -499:500 at index [770]
Stacktrace:
  [1] throw_boundserror(A::OffsetVector{Float64, Vector{Float64}}, I::Tuple{Int64})
    @ Base ./abstractarray.jl:691
  [2] checkbounds
    @ ./abstractarray.jl:656 [inlined]
  [3] setindex!
    @ ~/.julia/packages/OffsetArrays/aKeSs/src/OffsetArrays.jl:444 [inlined]
  [4] _setindex!
    @ ./abstractarray.jl:1334 [inlined]
  [5] setindex!
    @ ./abstractarray.jl:1315 [inlined]
  [6] sort!(vs::OffsetVector{Float64, Vector{Float64}}, lo::Int64, hi::Int64, ::SortingAlgorithms.RadixSortAlg, o::Base.Order.ForwardOrdering, ts::OffsetVector{Float64, Vector{Float64}})
    @ SortingAlgorithms ~/.julia/packages/SortingAlgorithms/PEcBU/src/SortingAlgorithms.jl:101
  [7] sort!
    @ ~/.julia/packages/SortingAlgorithms/PEcBU/src/SortingAlgorithms.jl:66 [inlined]
  [8] fpsort!
    @ ~/.julia/packages/SortingAlgorithms/PEcBU/src/SortingAlgorithms.jl:131 [inlined]
  [9] sort!
    @ ./sort.jl:1234 [inlined]
 [10] #sort!#8
    @ ./sort.jl:725 [inlined]
 [11] top-level scope
    @ REPL[5]:1
@nalimilan
Copy link
Contributor

Good catch. We should call Base.require_one_based_indexing at least.

@LilithHafner
Copy link
Member Author

@LilithHafnerBot bisect()

import Pkg
Pkg.add("OffsetArrays")
using SortingAlgorithms, OffsetArrays
sort!(OffsetArray(rand(1000), -500);  alg=RadixSort)

@LilithHafnerBot
Copy link

❗ Internal Error

Check the public logs for more information.

@LilithHafner
Copy link
Member Author

@LilithHafnerBot bisect()

import Pkg
Pkg.add("OffsetArrays")
using SortingAlgorithms, OffsetArrays
sort!(OffsetArray(rand(1000), -500);  alg=RadixSort)

@LilithHafnerBot
Copy link

✅ Bisect succeeded! The first new commit is 7179745

Commit Exit code stdout stderr
aa2b98d ❌ (1) Updating registry at ~/.julia/registries/General.toml⏎ Resolving package versions...⏎ ...
d0a9007 ❌ (1) Updating registry at ~/.julia/registries/General.toml⏎ Resolving package versions...⏎ ...
6174e49 ❌ (1) Updating registry at ~/.julia/registries/General.toml⏎ Resolving package versions...⏎ ...
a17c80c ❌ (1) Updating registry at ~/.julia/registries/General.toml⏎ Resolving package versions...⏎ ...
80c14f5 ❌ (1) Updating registry at ~/.julia/registries/General.toml⏎ Resolving package versions...⏎ ...
7179745 ✅ (0) [0.002757651757429369, 0.004082787124258691, 0.005861932577843065, 0.008842582089167439, 0.009131... ** Updating registry at ~/.julia/registries/General.toml⏎ Resolving package versions...⏎ ...**
10a3c6f ✅ (0) [0.0008313731242979294, 0.0012294832324717397, 0.0027805890341880968, 0.0034803665712288545, 0.00... Updating registry at ~/.julia/registries/General.toml⏎ Resolving package versions...⏎ I...

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 a pull request may close this issue.

3 participants