From 11c00f644eab5620da2737f2062a0035bac83310 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Thu, 8 Sep 2022 10:52:10 +0200 Subject: [PATCH 1/9] Fix `vcat` of sparse vectors with numbers --- src/sparsevector.jl | 3 ++- test/sparsevector.jl | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/sparsevector.jl b/src/sparsevector.jl index a9568455..d412ab97 100644 --- a/src/sparsevector.jl +++ b/src/sparsevector.jl @@ -1144,7 +1144,8 @@ const _SparseConcatGroup = Union{_DenseConcatGroup, _SparseConcatArrays, _Annota # Concatenations involving un/annotated sparse/special matrices/vectors should yield sparse arrays _makesparse(x::Number) = x -_makesparse(x::AbstractArray) = SparseMatrixCSC(issparse(x) ? x : sparse(x)) +_makesparse(x::AbstractVector) = convert(SparseVector, issparse(x) ? x : sparse(x))::SparseVector +_makesparse(x::AbstractMatrix) = convert(SparseMatrixCSC, issparse(x) ? x : sparse(x))::SparseMatrixCSC # `@constprop :aggressive` allows `dims` to be propagated as constant improving return type inference Base.@constprop :aggressive function Base._cat(dims, Xin::_SparseConcatGroup...) diff --git a/test/sparsevector.jl b/test/sparsevector.jl index 76d7446b..a5113d6c 100644 --- a/test/sparsevector.jl +++ b/test/sparsevector.jl @@ -544,6 +544,10 @@ end @test length(V) == m * n Vr = vec(Hr) @test Array(V) == Vr + Vnum = vcat(zero(Float64), A...) + @test Vnum isa SparseVector{Float64,Int} + @test length(Vnum) == m*n + 1 + @test Array(Vnum) == [0; Vr] end @testset "concatenation of sparse vectors with other types" begin From 2733ebcfab1c37668104a29b2fb07630bf11b94b Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Thu, 8 Sep 2022 11:57:58 +0200 Subject: [PATCH 2/9] test the other order --- test/sparsevector.jl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/sparsevector.jl b/test/sparsevector.jl index a5113d6c..d75f64e3 100644 --- a/test/sparsevector.jl +++ b/test/sparsevector.jl @@ -544,6 +544,10 @@ end @test length(V) == m * n Vr = vec(Hr) @test Array(V) == Vr + Vnum = vcat(A..., zero(Float64)) + @test Vnum isa SparseVector{Float64,Int} + @test length(Vnum) == m*n + 1 + @test Array(Vnum) == [Vr; 0] Vnum = vcat(zero(Float64), A...) @test Vnum isa SparseVector{Float64,Int} @test length(Vnum) == m*n + 1 From 1e2d3fc1ae66818e73f9c6e1668abae557e00427 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Thu, 8 Sep 2022 13:13:05 +0200 Subject: [PATCH 3/9] fix the first-element-isa-number case --- src/sparsevector.jl | 31 +++++++++++++++++++++---------- test/sparsevector.jl | 12 ++++++++---- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/sparsevector.jl b/src/sparsevector.jl index d412ab97..f6265bc0 100644 --- a/src/sparsevector.jl +++ b/src/sparsevector.jl @@ -1039,6 +1039,9 @@ complex(x::AbstractSparseVector) = ### Concatenation +_nnz(x::Number) = iszero(x) ? 0 : 1 +_nonzeroinds(x::Number, Ti) = iszero(x) ? Ti[] : Ti[1] +_nonzeros(x::Number) = # Without the first of these methods, horizontal concatenations of SparseVectors fall # back to the horizontal concatenation method that ensures that combinations of # sparse/special/dense matrix/vector types concatenate to SparseMatrixCSCs, instead @@ -1143,22 +1146,28 @@ const _Annotated_SparseConcatArrays = Union{_Triangular_SparseConcatArrays, _Sym const _SparseConcatGroup = Union{_DenseConcatGroup, _SparseConcatArrays, _Annotated_SparseConcatArrays} # Concatenations involving un/annotated sparse/special matrices/vectors should yield sparse arrays + +# the output array type is determined by the first element of the to be concatenated objects +# if this is a Number, the output would be dense by the fallback abstractarray.jl code (see cat_similar) +# so make sure that if that happens, the "array" is sparse (if more sparse arrays are involved, of course) +_sparse(x::Number) = sparse([1], [x], 1) +_sparse(A) = _makesparse(A) _makesparse(x::Number) = x _makesparse(x::AbstractVector) = convert(SparseVector, issparse(x) ? x : sparse(x))::SparseVector _makesparse(x::AbstractMatrix) = convert(SparseMatrixCSC, issparse(x) ? x : sparse(x))::SparseMatrixCSC # `@constprop :aggressive` allows `dims` to be propagated as constant improving return type inference Base.@constprop :aggressive function Base._cat(dims, Xin::_SparseConcatGroup...) - X = map(_makesparse, Xin) + X = (_sparse(first(Xin)), map(_makesparse, Base.tail(Xin))...) T = promote_eltype(Xin...) return Base._cat_t(dims, T, X...) end function hcat(Xin::_SparseConcatGroup...) - X = map(_makesparse, Xin) + X = (_sparse(first(Xin)), map(_makesparse, Base.tail(Xin))...) return cat(X..., dims=Val(2)) end function vcat(Xin::_SparseConcatGroup...) - X = map(_makesparse, Xin) + X = (_sparse(first(Xin)), map(_makesparse, Base.tail(Xin))...) return cat(X..., dims=Val(1)) end hvcat(rows::Tuple{Vararg{Int}}, X::_SparseConcatGroup...) = @@ -1171,8 +1180,10 @@ function _hvcat_rows((row1, rows...)::Tuple{Vararg{Int}}, X::_SparseConcatGroup. T = eltype(X::Tuple{Any,Vararg{Any}}) # inference of `getindex` may be imprecise in case `row1` is not const-propagated up # to here, so help inference with the following type-assertions + Xrow = X[1 : row1]::Tuple{typeof(X[1]),Vararg{T}} + Xsrow = (_sparse(first(Xrow)), map(_makesparse, Base.tail(Xrow))...) return ( - hcat(X[1 : row1]::Tuple{typeof(X[1]),Vararg{T}}...), + hcat(Xsrow...), _hvcat_rows(rows, X[row1+1:end]::Tuple{Vararg{T}}...)... ) end @@ -1192,9 +1203,9 @@ Concatenate along dimension 2. Return a SparseMatrixCSC object. the concatenation with specialized "sparse" matrix types from LinearAlgebra.jl automatically yielded sparse output even in the absence of any SparseArray argument. """ -sparse_hcat(Xin::Union{AbstractVecOrMat,Number}...) = cat(map(_makesparse, Xin)..., dims=Val(2)) +sparse_hcat(Xin::Union{AbstractVecOrMat,Number}...) = cat(_sparse(first(Xin)), map(_makesparse, Base.tail(Xin))..., dims=Val(2)) function sparse_hcat(X::Union{AbstractVecOrMat,UniformScaling,Number}...) - LinearAlgebra._hcat(X...; array_type = SparseMatrixCSC) + LinearAlgebra._hcat(_sparse(first(X)), map(_makesparse, Base.tail(X))...; array_type = SparseMatrixCSC) end """ @@ -1207,9 +1218,9 @@ Concatenate along dimension 1. Return a SparseMatrixCSC object. the concatenation with specialized "sparse" matrix types from LinearAlgebra.jl automatically yielded sparse output even in the absence of any SparseArray argument. """ -sparse_vcat(Xin::Union{AbstractVecOrMat,Number}...) = cat(map(_makesparse, Xin)..., dims=Val(1)) +sparse_vcat(Xin::Union{AbstractVecOrMat,Number}...) = cat(_sparse(first(Xin)), map(_makesparse, Base.tail(Xin))..., dims=Val(1)) function sparse_vcat(X::Union{AbstractVecOrMat,UniformScaling,Number}...) - LinearAlgebra._vcat(X...; array_type = SparseMatrixCSC) + LinearAlgebra._vcat(_sparse(first(X)), map(_makesparse, Base.tail(X))...; array_type = SparseMatrixCSC) end """ @@ -1225,10 +1236,10 @@ arguments to concatenate in each block row. automatically yielded sparse output even in the absence of any SparseArray argument. """ function sparse_hvcat(rows::Tuple{Vararg{Int}}, Xin::Union{AbstractVecOrMat,Number}...) - hvcat(rows, map(_makesparse, Xin)...) + hvcat(rows, _sparse(first(Xin)), map(_makesparse, Base.tail(Xin))...) end function sparse_hvcat(rows::Tuple{Vararg{Int}}, X::Union{AbstractVecOrMat,UniformScaling,Number}...) - LinearAlgebra._hvcat(rows, X...; array_type = SparseMatrixCSC) + LinearAlgebra._hvcat(rows, _sparse(first(X)), map(_makesparse, Base.tail(X))...; array_type = SparseMatrixCSC) end ### math functions diff --git a/test/sparsevector.jl b/test/sparsevector.jl index d75f64e3..353633b5 100644 --- a/test/sparsevector.jl +++ b/test/sparsevector.jl @@ -545,13 +545,17 @@ end Vr = vec(Hr) @test Array(V) == Vr Vnum = vcat(A..., zero(Float64)) + Vnum2 = sparse_vcat(map(Array, A)..., zero(Float64)) @test Vnum isa SparseVector{Float64,Int} - @test length(Vnum) == m*n + 1 - @test Array(Vnum) == [Vr; 0] + @test Vnum2 isa SparseVector{Float64,Int} + @test length(Vnum) == length(Vnum2) == m*n + 1 + @test Array(Vnum) == Array(Vnum2) == [Vr; 0] Vnum = vcat(zero(Float64), A...) + Vnum = sparse_vcat(zero(Float64), map(Array, A)...) @test Vnum isa SparseVector{Float64,Int} - @test length(Vnum) == m*n + 1 - @test Array(Vnum) == [0; Vr] + @test Vnum2 isa SparseVector{Float64,Int} + @test length(Vnum) == length(Vnum2) == m*n + 1 + @test Array(Vnum) == Array(Vnum2) == [0; Vr] end @testset "concatenation of sparse vectors with other types" begin From a5f0d77fa92cfe66edcdb5b76885bcc1c47a46fe Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Thu, 8 Sep 2022 13:35:42 +0200 Subject: [PATCH 4/9] cleanup --- src/sparsevector.jl | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/sparsevector.jl b/src/sparsevector.jl index f6265bc0..88117f77 100644 --- a/src/sparsevector.jl +++ b/src/sparsevector.jl @@ -1039,9 +1039,6 @@ complex(x::AbstractSparseVector) = ### Concatenation -_nnz(x::Number) = iszero(x) ? 0 : 1 -_nonzeroinds(x::Number, Ti) = iszero(x) ? Ti[] : Ti[1] -_nonzeros(x::Number) = # Without the first of these methods, horizontal concatenations of SparseVectors fall # back to the horizontal concatenation method that ensures that combinations of # sparse/special/dense matrix/vector types concatenate to SparseMatrixCSCs, instead @@ -1150,7 +1147,7 @@ const _SparseConcatGroup = Union{_DenseConcatGroup, _SparseConcatArrays, _Annota # the output array type is determined by the first element of the to be concatenated objects # if this is a Number, the output would be dense by the fallback abstractarray.jl code (see cat_similar) # so make sure that if that happens, the "array" is sparse (if more sparse arrays are involved, of course) -_sparse(x::Number) = sparse([1], [x], 1) +_sparse(x::Number) = sparsevec([1], [x], 1) _sparse(A) = _makesparse(A) _makesparse(x::Number) = x _makesparse(x::AbstractVector) = convert(SparseVector, issparse(x) ? x : sparse(x))::SparseVector From 68fd02d09e4ad1d2469a80db08a8387d31d50477 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Thu, 8 Sep 2022 14:36:43 +0200 Subject: [PATCH 5/9] revert the hvcat-related changes --- src/sparsevector.jl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/sparsevector.jl b/src/sparsevector.jl index 88117f77..0f8916ea 100644 --- a/src/sparsevector.jl +++ b/src/sparsevector.jl @@ -1177,10 +1177,8 @@ function _hvcat_rows((row1, rows...)::Tuple{Vararg{Int}}, X::_SparseConcatGroup. T = eltype(X::Tuple{Any,Vararg{Any}}) # inference of `getindex` may be imprecise in case `row1` is not const-propagated up # to here, so help inference with the following type-assertions - Xrow = X[1 : row1]::Tuple{typeof(X[1]),Vararg{T}} - Xsrow = (_sparse(first(Xrow)), map(_makesparse, Base.tail(Xrow))...) return ( - hcat(Xsrow...), + hcat(X[1 : row1]::Tuple{typeof(X[1]),Vararg{T}}...), _hvcat_rows(rows, X[row1+1:end]::Tuple{Vararg{T}}...)... ) end From fb290278e8d7dfbf7455e9963e2b5d3f0ba50378 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Thu, 8 Sep 2022 15:10:41 +0200 Subject: [PATCH 6/9] fix typo --- test/sparsevector.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/sparsevector.jl b/test/sparsevector.jl index 353633b5..0f747bdf 100644 --- a/test/sparsevector.jl +++ b/test/sparsevector.jl @@ -551,7 +551,7 @@ end @test length(Vnum) == length(Vnum2) == m*n + 1 @test Array(Vnum) == Array(Vnum2) == [Vr; 0] Vnum = vcat(zero(Float64), A...) - Vnum = sparse_vcat(zero(Float64), map(Array, A)...) + Vnum2 = sparse_vcat(zero(Float64), map(Array, A)...) @test Vnum isa SparseVector{Float64,Int} @test Vnum2 isa SparseVector{Float64,Int} @test length(Vnum) == length(Vnum2) == m*n + 1 From d4d3545705b910b30c48fe3b1e256ed2539d20ab Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Thu, 8 Sep 2022 15:38:31 +0200 Subject: [PATCH 7/9] add a hvcat test --- test/sparsevector.jl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/sparsevector.jl b/test/sparsevector.jl index 0f747bdf..cf2b0e9e 100644 --- a/test/sparsevector.jl +++ b/test/sparsevector.jl @@ -556,6 +556,10 @@ end @test Vnum2 isa SparseVector{Float64,Int} @test length(Vnum) == length(Vnum2) == m*n + 1 @test Array(Vnum) == Array(Vnum2) == [0; Vr] + # case with rowwise a Number as first element, should still yield a sparse matrix + x = sparsevec([1], [3.0], 1) + X = [3.0 x; 3.0 x] + @test issparse(X) end @testset "concatenation of sparse vectors with other types" begin From 4c77c0cd8797cdb61f0da29a5c42ffa8586eda35 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Mon, 12 Sep 2022 16:25:53 +0200 Subject: [PATCH 8/9] test inference? --- test/sparsevector.jl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/sparsevector.jl b/test/sparsevector.jl index cf2b0e9e..32344114 100644 --- a/test/sparsevector.jl +++ b/test/sparsevector.jl @@ -544,21 +544,21 @@ end @test length(V) == m * n Vr = vec(Hr) @test Array(V) == Vr - Vnum = vcat(A..., zero(Float64)) - Vnum2 = sparse_vcat(map(Array, A)..., zero(Float64)) + Vnum = @inferred vcat(A..., zero(Float64)) + Vnum2 = @inferred sparse_vcat(map(Array, A)..., zero(Float64)) @test Vnum isa SparseVector{Float64,Int} @test Vnum2 isa SparseVector{Float64,Int} @test length(Vnum) == length(Vnum2) == m*n + 1 @test Array(Vnum) == Array(Vnum2) == [Vr; 0] - Vnum = vcat(zero(Float64), A...) - Vnum2 = sparse_vcat(zero(Float64), map(Array, A)...) + Vnum = @inferred vcat(zero(Float64), A...) + Vnum2 = @inferred sparse_vcat(zero(Float64), map(Array, A)...) @test Vnum isa SparseVector{Float64,Int} @test Vnum2 isa SparseVector{Float64,Int} @test length(Vnum) == length(Vnum2) == m*n + 1 @test Array(Vnum) == Array(Vnum2) == [0; Vr] # case with rowwise a Number as first element, should still yield a sparse matrix x = sparsevec([1], [3.0], 1) - X = [3.0 x; 3.0 x] + X = @inferred hvcat((2, 2), 3.0, x, 3.0, x) @test issparse(X) end From 9e47ca31abbd58d4fac85c2c68742b4dfecd3314 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Mon, 12 Sep 2022 16:55:49 +0200 Subject: [PATCH 9/9] Revert "test inference?" This reverts commit 4c77c0cd8797cdb61f0da29a5c42ffa8586eda35. --- test/sparsevector.jl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/sparsevector.jl b/test/sparsevector.jl index 32344114..cf2b0e9e 100644 --- a/test/sparsevector.jl +++ b/test/sparsevector.jl @@ -544,21 +544,21 @@ end @test length(V) == m * n Vr = vec(Hr) @test Array(V) == Vr - Vnum = @inferred vcat(A..., zero(Float64)) - Vnum2 = @inferred sparse_vcat(map(Array, A)..., zero(Float64)) + Vnum = vcat(A..., zero(Float64)) + Vnum2 = sparse_vcat(map(Array, A)..., zero(Float64)) @test Vnum isa SparseVector{Float64,Int} @test Vnum2 isa SparseVector{Float64,Int} @test length(Vnum) == length(Vnum2) == m*n + 1 @test Array(Vnum) == Array(Vnum2) == [Vr; 0] - Vnum = @inferred vcat(zero(Float64), A...) - Vnum2 = @inferred sparse_vcat(zero(Float64), map(Array, A)...) + Vnum = vcat(zero(Float64), A...) + Vnum2 = sparse_vcat(zero(Float64), map(Array, A)...) @test Vnum isa SparseVector{Float64,Int} @test Vnum2 isa SparseVector{Float64,Int} @test length(Vnum) == length(Vnum2) == m*n + 1 @test Array(Vnum) == Array(Vnum2) == [0; Vr] # case with rowwise a Number as first element, should still yield a sparse matrix x = sparsevec([1], [3.0], 1) - X = @inferred hvcat((2, 2), 3.0, x, 3.0, x) + X = [3.0 x; 3.0 x] @test issparse(X) end