JuliaSparse / SuiteSparseGraphBLAS.jl

Sparse, General Linear Algebra for Graphs!
MIT License
102 stars 17 forks source link

Can't use transposed GBVector for subassign! #85

Closed andreyz4k closed 2 years ago

andreyz4k commented 2 years ago

There are two cases:

  1. If vector has missing values

    
    julia> A = GBMatrix([1,1,2,2,3,4,4,5,6,7,7,7], [2,4,5,7,6,1,3,6,3,3,4,5], [1:12...])
    7x7 GraphBLAS int64_t matrix, bitmap by row
    12 entries, memory: 649 bytes
    
    (1,2)   1
    (1,4)   2
    (2,5)   3
    (2,7)   4
    (3,6)   5
    (4,1)   6
    (4,3)   7
    (5,6)   8
    (6,3)   9
    (7,3)   10
    (7,4)   11
    (7,5)   12

julia> B = GBVector([3, 4, 5, 6, 7], [3, 4, 5, 6, 7]) 7x1 GraphBLAS int64_t matrix, bitmap by col 5 entries, memory: 272 bytes

(3,1)   3
(4,1)   4
(5,1)   5
(6,1)   6
(7,1)   7

julia> subassign!(A, B', 3, :) ERROR: TypeError: in typeassert, expected Int64, got a value of type Nothing Stacktrace: [1] getindex @ /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/LinearAlgebra/src/adjtrans.jl:178 [inlined] [2] copyto_unaliased!(deststyle::IndexLinear, dest::Matrix{Int64}, srcstyle::IndexLinear, src::LinearAlgebra.Transpose{Int64, GBVector{Int64, Nothing}}) @ Base ./abstractarray.jl:1018 [3] copyto!(dest::Matrix{Int64}, src::LinearAlgebra.Transpose{Int64, GBVector{Int64, Nothing}}) @ Base ./abstractarray.jl:998 [4] _collect_indices @ ./array.jl:667 [inlined] [5] collect @ ./array.jl:651 [inlined] [6] subassign!(C::GBMatrix{Int64, Nothing}, x::LinearAlgebra.Transpose{Int64, GBVector{Int64, Nothing}}, I::Int64, J::Function; mask::Nothing, accum::Nothing, desc::Nothing) @ SuiteSparseGraphBLAS ~/.julia/packages/SuiteSparseGraphBLAS/5AOj3/src/abstractgbarray.jl:362 [7] subassign!(C::GBMatrix{Int64, Nothing}, x::LinearAlgebra.Transpose{Int64, GBVector{Int64, Nothing}}, I::Int64, J::Function) @ SuiteSparseGraphBLAS ~/.julia/packages/SuiteSparseGraphBLAS/5AOj3/src/abstractgbarray.jl:362 [8] top-level scope @ REPL[140]:1

2. If there is a mask

julia> B = GBVector([1, 2, 3, 4, 5, 6, 7], [1, 2, 3, 4, 5, 6, 7]) 7x1 GraphBLAS int64_t matrix, full by col 7 entries, memory: 264 bytes

(1,1)   1
(2,1)   2
(3,1)   3
(4,1)   4
(5,1)   5
(6,1)   6
(7,1)   7

julia> subassign!(A, B', 3, :; mask=B') ERROR: DimensionMismatch("") Stacktrace: [1] subassign!(C::GBMatrix{Int64, Nothing}, A::GBMatrix{Int64, Nothing}, I::Int64, J::Function; mask::LinearAlgebra.Transpose{Int64, GBVector{Int64, Nothing}}, accum::Nothing, desc::Nothing) @ SuiteSparseGraphBLAS ~/.julia/packages/SuiteSparseGraphBLAS/5AOj3/src/abstractgbarray.jl:338 [2] subassign!(C::GBMatrix{Int64, Nothing}, x::LinearAlgebra.Transpose{Int64, GBVector{Int64, Nothing}}, I::Int64, J::Function; mask::LinearAlgebra.Transpose{Int64, GBVector{Int64, Nothing}}, accum::Nothing, desc::Nothing) @ SuiteSparseGraphBLAS ~/.julia/packages/SuiteSparseGraphBLAS/5AOj3/src/abstractgbarray.jl:364 [3] top-level scope @ REPL[143]:1

rayegun commented 2 years ago

Thanks for finding all of these. It's difficult to ensure I get everything when mapping the Julia array interface to C.

Some of these are already fixed by an in progress rewrite. I will fix and add tests for the rest.

andreyz4k commented 2 years ago

I've updated to the last master version and still get the exact same error. Moreover, since A[3, :] now returns a GBVector instead of a 1xn GBMatrix, it makes it impossible to workaround with using GBMatrix everywhere.

rayegun commented 2 years ago

I haven't yet fixed the print error for the first example, that I will need to do this weekend (I believe the primary error there should be fixed, though).

The second issue is a bit more complex, there is no supported method to use the transpose of a mask currently, and the behavior for a matrix is actually incorrect I believe (somehow it is just passing a pointer to the transposed mask, rather than erroring). I will have to update mask handling (something I've been meaning to do anyway).

rayegun commented 2 years ago

@andreyz4k could you test current master for this now?

There was an unsafe fallback in Julia Base that I did not handle correctly. So mask = B' was being converted to mask=B.

On the topic of GBVector vs GBMatrix I will ensure that GBVector can be used everywhere it should be. They are the same time underneath, so I just need to check that methods are sufficiently broad.

andreyz4k commented 2 years ago

Yes, it works now. But there is still a tricky case, introduced with your recent changes.

Before, when I took A[:, 1] I got Nx1 matrix, and when I took A[1, :] I got 1xM matrix.

Now, in both cases, I receive a vector of length N or M, which is basically Nx1 matrix. So in the second case, I need to transpose it again by hand to convert it back to 1xN size. So if I'll try to do A[2, :] = A[1, :], I'll get a DimensionMismatch error.

rayegun commented 2 years ago

Okay I need to be sure Vectors will be properly resized. I switched to Vectors if there's a : involved since that's what Julia does, and it should have minimal performance impact.

rayegun commented 2 years ago

@andreyz4k is everything working for you now on current master?

andreyz4k commented 2 years ago

I've tried to update and got this error on compilation

julia> import Pkg; Pkg.precompile()
Precompiling project...
  ✗ SuiteSparseGraphBLAS
  0 dependencies successfully precompiled in 44 seconds. 37 already precompiled.

ERROR: The following 1 direct dependency failed to precompile:

SuiteSparseGraphBLAS [c2e53296-7b14-11e9-1210-bddfa8111e1d]

Failed to precompile SuiteSparseGraphBLAS [c2e53296-7b14-11e9-1210-bddfa8111e1d] to /Users/andreyz4k/.julia/compiled/v1.8/SuiteSparseGraphBLAS/jl_ns4Eiv.
ERROR: LoadError: UndefVarError: Structural not defined
Stacktrace:
 [1] top-level scope
   @ ~/.julia/packages/SuiteSparseGraphBLAS/MTl3G/src/operations/operationutils.jl:21
 [2] include(mod::Module, _path::String)
   @ Base ./Base.jl:419
 [3] include(x::String)
   @ SuiteSparseGraphBLAS ~/.julia/packages/SuiteSparseGraphBLAS/MTl3G/src/SuiteSparseGraphBLAS.jl:1
 [4] top-level scope
   @ ~/.julia/packages/SuiteSparseGraphBLAS/MTl3G/src/SuiteSparseGraphBLAS.jl:65
 [5] include
   @ ./Base.jl:419 [inlined]
 [6] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt64}}, source::Nothing)
   @ Base ./loading.jl:1554
 [7] top-level scope
   @ stdin:1
in expression starting at /Users/andreyz4k/.julia/packages/SuiteSparseGraphBLAS/MTl3G/src/operations/operationutils.jl:21
in expression starting at /Users/andreyz4k/.julia/packages/SuiteSparseGraphBLAS/MTl3G/src/SuiteSparseGraphBLAS.jl:1
in expression starting at stdin:1
Stacktrace:
 [1] pkgerror(msg::String)
   @ Pkg.Types /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Pkg/src/Types.jl:67
 [2] precompile(ctx::Pkg.Types.Context, pkgs::Vector{String}; internal_call::Bool, strict::Bool, warn_loaded::Bool, already_instantiated::Bool, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ Pkg.API /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Pkg/src/API.jl:1427
 [3] precompile
   @ /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Pkg/src/API.jl:1058 [inlined]
 [4] #precompile#225
   @ /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Pkg/src/API.jl:1057 [inlined]
 [5] precompile (repeats 2 times)
   @ /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Pkg/src/API.jl:1057 [inlined]
 [6] top-level scope
   @ REPL[20]:1
andreyz4k commented 2 years ago

And after I fixed this, I got an error that reduce(+, A, dims=1) hangs forever in infinite loop.