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

Wrong result for sum of SparseVector with UInt indices #285

Open
kalmarek opened this issue Oct 31, 2022 · 2 comments
Open

Wrong result for sum of SparseVector with UInt indices #285

kalmarek opened this issue Oct 31, 2022 · 2 comments

Comments

@kalmarek
Copy link

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
#262
so I don't have a good solution for this one.

@dkarrasch
Copy link
Member

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.

@kalmarek
Copy link
Author

kalmarek commented Oct 31, 2022

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

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

No branches or pull requests

2 participants