SparseArrays.jl icon indicating copy to clipboard operation
SparseArrays.jl copied to clipboard

Wrong result for sum of SparseVector with UInt indices

Open kalmarek opened this issue 2 years ago • 2 comments

julia> using SparseArrays

julia> x = SparseVector{Int, UInt8}(6, UInt8[1], [-1])
6-element SparseVector{Int64, UInt8} with 1 stored entry:
  [1]  =  -1

julia> sum(x)
-1

julia> versioninfo()
Julia Version 1.6.7
Commit 3b76b25b64 (2022-07-19 15:11 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: AMD Ryzen 7 PRO 4750U with Radeon Graphics
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, znver2)
Environment:
  JULIA_NUM_THREADS = 8

On the other hand:

julia> using SparseArrays

julia> x = SparseVector{Int, UInt8}(6, UInt8[1], [-1])
6-element SparseVector{Int64, UInt8} with 1 stored entry:
  [1]  =  -1

julia> sum(x)
0xffffffffffffffff

julia> versioninfo()
Julia Version 1.8.2
Commit 36034abf260 (2022-09-29 15:21 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 16 × AMD Ryzen 7 PRO 4750U with Radeon Graphics
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-13.0.1 (ORCJIT, znver2)
  Threads: 8 on 16 virtual cores
Environment:
  JULIA_NUM_THREADS = 8

This is because on julia-1.6.7 size(x) is a Tuple{Int64} but on julia-1.8.2 it's Tuple{UInt8}.

as a bonus now sum is type unstable:

julia> x = SparseVector{Int, UInt8}(6, UInt8[1], [-1]); sum(x)
0xffffffffffffffff

julia> x = SparseVector{Int, UInt8}(1, UInt8[1], [-1]); sum(x)
-1

I understand that preserving the type of indices is useful for a very long sp vectors as per https://github.com/JuliaSparse/SparseArrays.jl/pull/262 so I don't have a good solution for this one.

kalmarek avatar Oct 31 '22 13:10 kalmarek

That seems fixed on nightly/v1.9:

julia> using SparseArrays

julia> x = SparseVector{Int, UInt8}(6, UInt8[1], [-1])
6-element SparseVector{Int64, UInt8} with 1 stored entry:
  [1]  =  -1

julia> sum(x)
-1

julia> versioninfo()
Julia Version 1.9.0-DEV.1680
Commit 822c9cf2d2* (2022-10-28 08:45 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin21.6.0)
  CPU: 4 × Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, skylake)
  Threads: 1 on 4 virtual cores

julia> x = SparseVector{Int, UInt8}(6, UInt8[1], [-1]); sum(x)
-1

julia> x = SparseVector{Int, UInt8}(1, UInt8[1], [-1]); sum(x)
-1

There has been some rework of sparse reductions, in particular #63 (may have) fixed it. The only question is whether we should backport that fix to v1.8.

dkarrasch avatar Oct 31 '22 16:10 dkarrasch

I consider this a serious bug:

julia> x = SparseVector{Int, UInt8}(6, UInt8[1], [-1]); sum(x) == -1
false

so a backport would be nice; however if this is too much intertwined what would be a sensible workaround I could apply to in package? (pirating is not sensible ;)

btw. this mentod (removed in #63) gives correct results: https://github.com/JuliaSparse/SparseArrays.jl/pull/63/files#diff-4921b1a62380bc3535483810d7b563c0ac958f95beb493ac2aa374dc7a9672f8L1405

kalmarek avatar Oct 31 '22 19:10 kalmarek