From 87fae9975a55de5949992b6221ae09fcff78a522 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Wed, 14 Oct 2020 10:27:58 +0200 Subject: [PATCH 1/7] Fix sparse Subarray fill! --- stdlib/SparseArrays/src/sparsematrix.jl | 11 +++++++---- stdlib/SparseArrays/test/sparse.jl | 6 ++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/stdlib/SparseArrays/src/sparsematrix.jl b/stdlib/SparseArrays/src/sparsematrix.jl index 967d93a247c23..b398cd84b7de1 100644 --- a/stdlib/SparseArrays/src/sparsematrix.jl +++ b/stdlib/SparseArrays/src/sparsematrix.jl @@ -2588,7 +2588,7 @@ function Base.fill!(V::SubArray{Tv, <:Any, <:AbstractSparseMatrixCSC{Tv}, <:Tupl end end """ -Helper method for immediately preceding setindex! method. For all (i,j) such that i in I and +Helper method for immediately preceding fill! method. For all (i,j) such that i in I and j in J, assigns zero to A[i,j] if A[i,j] is a presently-stored entry, and otherwise does nothing. """ function _spsetz_setindex!(A::AbstractSparseMatrixCSC, @@ -2624,7 +2624,7 @@ function _spsetz_setindex!(A::AbstractSparseMatrixCSC, end end """ -Helper method for immediately preceding setindex! method. For all (i,j) such that i in I +Helper method for immediately preceding fill! method. For all (i,j) such that i in I and j in J, assigns x to A[i,j] if A[i,j] is a presently-stored entry, and allocates and assigns x to A[i,j] if A[i,j] is not presently stored. """ @@ -2671,7 +2671,7 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, while true old_row = rowval[old_ptr] - new_row = I[new_ptr] + new_row = _getrowinds(I, new_ptr) if old_row < new_row rowvalA[rowidx] = old_row nzvalA[rowidx] = nzval[old_ptr] @@ -2704,7 +2704,7 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, resize!(nzvalA, nnzA) end r = rowidx:(rowidx+(new_stop-new_ptr)) - rowvalA[r] .= I[new_ptr:new_stop] + rowvalA[r] .= _getrowinds(I, new_ptr:new_stop) for rr in r nzvalA[rr] = x end @@ -2739,6 +2739,9 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, return A end +_getrowinds(I::Number, range) = I +_getrowinds(I::AbstractVector{<:Integer}, range) = I[range] + # Nonscalar A[I,J] = B: Convert B to a SparseMatrixCSC of the appropriate shape first _to_same_csc(::AbstractSparseMatrixCSC{Tv, Ti}, V::AbstractMatrix, I...) where {Tv,Ti} = convert(SparseMatrixCSC{Tv,Ti}, V) _to_same_csc(::AbstractSparseMatrixCSC{Tv, Ti}, V::AbstractVector, I...) where {Tv,Ti} = convert(SparseMatrixCSC{Tv,Ti}, reshape(V, map(length, I))) diff --git a/stdlib/SparseArrays/test/sparse.jl b/stdlib/SparseArrays/test/sparse.jl index 9fdeddea113be..7c60ac1213a31 100644 --- a/stdlib/SparseArrays/test/sparse.jl +++ b/stdlib/SparseArrays/test/sparse.jl @@ -1057,6 +1057,12 @@ end @test a[1,:] == sparse([1; 1; 3:10]) a[1:0,2] .= 1 @test a[:,2] == sparse([1:10;]) + a[1,2:3] .= 1 + @test a[1,:] == sparse([1; 1; 1; 4:10]) + a[2:4,2] .= 1 + @test a[:,2] == sparse([1; 1; 1; 1; 5:10]) + a[2:4,2:3] .= 3 + @test nnz(a) == 22 @test_throws BoundsError a[:,11] = spzeros(10,1) @test_throws BoundsError a[11,:] = spzeros(1,10) From 97ee86c19a47710f4e24e6c2146e86c6c9b38bf9 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Wed, 14 Oct 2020 10:42:36 +0200 Subject: [PATCH 2/7] put back scalar indexing --- stdlib/SparseArrays/src/sparsematrix.jl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/stdlib/SparseArrays/src/sparsematrix.jl b/stdlib/SparseArrays/src/sparsematrix.jl index b398cd84b7de1..484c6675d5392 100644 --- a/stdlib/SparseArrays/src/sparsematrix.jl +++ b/stdlib/SparseArrays/src/sparsematrix.jl @@ -2671,7 +2671,7 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, while true old_row = rowval[old_ptr] - new_row = _getrowinds(I, new_ptr) + new_row = I[new_ptr] if old_row < new_row rowvalA[rowidx] = old_row nzvalA[rowidx] = nzval[old_ptr] @@ -2739,7 +2739,6 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, return A end -_getrowinds(I::Number, range) = I _getrowinds(I::AbstractVector{<:Integer}, range) = I[range] # Nonscalar A[I,J] = B: Convert B to a SparseMatrixCSC of the appropriate shape first From 348523efc8f2ac9a1fd859c3115cc4586e84cf2b Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Wed, 14 Oct 2020 16:19:51 +0200 Subject: [PATCH 3/7] add back Number method --- stdlib/SparseArrays/src/sparsematrix.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stdlib/SparseArrays/src/sparsematrix.jl b/stdlib/SparseArrays/src/sparsematrix.jl index 484c6675d5392..fc01df2439368 100644 --- a/stdlib/SparseArrays/src/sparsematrix.jl +++ b/stdlib/SparseArrays/src/sparsematrix.jl @@ -2738,7 +2738,8 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, end return A end - +# ignore indexing into a Number by range +_getrowinds(I::Number, range) = I _getrowinds(I::AbstractVector{<:Integer}, range) = I[range] # Nonscalar A[I,J] = B: Convert B to a SparseMatrixCSC of the appropriate shape first From 636c43a611ee52f025df62c7ca9210e60b572538 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Wed, 14 Oct 2020 16:20:15 +0200 Subject: [PATCH 4/7] diversify tests --- stdlib/SparseArrays/test/sparse.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/stdlib/SparseArrays/test/sparse.jl b/stdlib/SparseArrays/test/sparse.jl index 7c60ac1213a31..aee7799f83264 100644 --- a/stdlib/SparseArrays/test/sparse.jl +++ b/stdlib/SparseArrays/test/sparse.jl @@ -1057,12 +1057,12 @@ end @test a[1,:] == sparse([1; 1; 3:10]) a[1:0,2] .= 1 @test a[:,2] == sparse([1:10;]) - a[1,2:3] .= 1 - @test a[1,:] == sparse([1; 1; 1; 4:10]) - a[2:4,2] .= 1 - @test a[:,2] == sparse([1; 1; 1; 1; 5:10]) - a[2:4,2:3] .= 3 - @test nnz(a) == 22 + a[3,2:3] .= 1 # one stored, one new value + @test a[3,2:3] == sparse([1; 1]) + a[5:6,1] .= 1 # only new values + @test a[:,1] == sparse([1; 0; 0; 0; 1; 1; 0; 0; 0; 0;]) + a[2:4,2:3] .= 3 # two ranges + @test nnz(a) == 24 @test_throws BoundsError a[:,11] = spzeros(10,1) @test_throws BoundsError a[11,:] = spzeros(1,10) From a9b5f17c061366770535e4f15dd2b4b736e23a35 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Wed, 14 Oct 2020 18:05:30 +0200 Subject: [PATCH 5/7] Update stdlib/SparseArrays/src/sparsematrix.jl Co-authored-by: Matt Bauman --- stdlib/SparseArrays/src/sparsematrix.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/SparseArrays/src/sparsematrix.jl b/stdlib/SparseArrays/src/sparsematrix.jl index fc01df2439368..68a7069b35522 100644 --- a/stdlib/SparseArrays/src/sparsematrix.jl +++ b/stdlib/SparseArrays/src/sparsematrix.jl @@ -2740,7 +2740,7 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, end # ignore indexing into a Number by range _getrowinds(I::Number, range) = I -_getrowinds(I::AbstractVector{<:Integer}, range) = I[range] +_getrowinds(I::AbstractVector{<:Integer}, range) = @inbounds I[range] # Nonscalar A[I,J] = B: Convert B to a SparseMatrixCSC of the appropriate shape first _to_same_csc(::AbstractSparseMatrixCSC{Tv, Ti}, V::AbstractMatrix, I...) where {Tv,Ti} = convert(SparseMatrixCSC{Tv,Ti}, V) From e2aa2717c02abe485242d1431207c7cff18cd3fc Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Mon, 19 Oct 2020 19:26:39 +0200 Subject: [PATCH 6/7] more concise version --- stdlib/SparseArrays/src/sparsematrix.jl | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/stdlib/SparseArrays/src/sparsematrix.jl b/stdlib/SparseArrays/src/sparsematrix.jl index 68a7069b35522..1104bb1ef3619 100644 --- a/stdlib/SparseArrays/src/sparsematrix.jl +++ b/stdlib/SparseArrays/src/sparsematrix.jl @@ -2704,7 +2704,7 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, resize!(nzvalA, nnzA) end r = rowidx:(rowidx+(new_stop-new_ptr)) - rowvalA[r] .= _getrowinds(I, new_ptr:new_stop) + rowvalA[r] .= I <: Number ? I : I[new_ptr:new_stop] for rr in r nzvalA[rr] = x end @@ -2738,9 +2738,6 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, end return A end -# ignore indexing into a Number by range -_getrowinds(I::Number, range) = I -_getrowinds(I::AbstractVector{<:Integer}, range) = @inbounds I[range] # Nonscalar A[I,J] = B: Convert B to a SparseMatrixCSC of the appropriate shape first _to_same_csc(::AbstractSparseMatrixCSC{Tv, Ti}, V::AbstractMatrix, I...) where {Tv,Ti} = convert(SparseMatrixCSC{Tv,Ti}, V) From f07b96b14fd27ffef3c34056b2eafae584987dfd Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Tue, 20 Oct 2020 09:12:47 +0200 Subject: [PATCH 7/7] Update stdlib/SparseArrays/src/sparsematrix.jl Co-authored-by: Matt Bauman --- stdlib/SparseArrays/src/sparsematrix.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/SparseArrays/src/sparsematrix.jl b/stdlib/SparseArrays/src/sparsematrix.jl index 1104bb1ef3619..7bbf5a28828c2 100644 --- a/stdlib/SparseArrays/src/sparsematrix.jl +++ b/stdlib/SparseArrays/src/sparsematrix.jl @@ -2704,7 +2704,7 @@ function _spsetnz_setindex!(A::AbstractSparseMatrixCSC{Tv}, x::Tv, resize!(nzvalA, nnzA) end r = rowidx:(rowidx+(new_stop-new_ptr)) - rowvalA[r] .= I <: Number ? I : I[new_ptr:new_stop] + rowvalA[r] .= I isa Number ? I : I[new_ptr:new_stop] for rr in r nzvalA[rr] = x end