Skip to content

Commit 9156082

Browse files
tkfViralBShah
authored andcommitted
Refactoring: Use accessor methods when manipulating sparse matrices/vectors (#32953)
* Use accessor methods when touching SparseMatrixCSC
1 parent 828a2ec commit 9156082

File tree

14 files changed

+759
-740
lines changed

14 files changed

+759
-740
lines changed

stdlib/SparseArrays/src/higherorderfns.jl

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import Base: map, map!, broadcast, copy, copyto!
99
using Base: front, tail, to_shape
1010
using ..SparseArrays: SparseVector, SparseMatrixCSC, AbstractSparseVector,
1111
AbstractSparseMatrix, AbstractSparseArray, indtype, nnz, nzrange,
12-
SparseVectorUnion, AdjOrTransSparseVectorUnion, nonzeroinds, nonzeros
12+
SparseVectorUnion, AdjOrTransSparseVectorUnion, nonzeroinds, nonzeros, rowvals, getcolptr
1313
using Base.Broadcast: BroadcastStyle, Broadcasted, flatten
1414
using LinearAlgebra
1515

@@ -108,24 +108,24 @@ const SpBroadcasted2{Style<:SPVM,Axes,F,Args<:Tuple{SparseVecOrMat,SparseVecOrMa
108108
# sufficient for the purposes of map[!]/broadcast[!]. This interface treats sparse vectors
109109
# as n-by-one sparse matrices which, though technically incorrect, is how broacast[!] views
110110
# sparse vectors in practice.
111-
@inline numrows(A::SparseVector) = A.n
112-
@inline numrows(A::SparseMatrixCSC) = A.m
111+
@inline numrows(A::SparseVector) = length(A)
112+
@inline numrows(A::SparseMatrixCSC) = size(A, 1)
113113
@inline numcols(A::SparseVector) = 1
114-
@inline numcols(A::SparseMatrixCSC) = A.n
114+
@inline numcols(A::SparseMatrixCSC) = size(A, 2)
115115
# numrows and numcols respectively yield size(A, 1) and size(A, 2), but avoid a branch
116116
@inline columns(A::SparseVector) = 1
117-
@inline columns(A::SparseMatrixCSC) = 1:A.n
118-
@inline colrange(A::SparseVector, j) = 1:length(A.nzind)
117+
@inline columns(A::SparseMatrixCSC) = 1:size(A, 2)
118+
@inline colrange(A::SparseVector, j) = 1:length(nonzeroinds(A))
119119
@inline colrange(A::SparseMatrixCSC, j) = nzrange(A, j)
120120
@inline colstartind(A::SparseVector, j) = one(indtype(A))
121-
@inline colboundind(A::SparseVector, j) = convert(indtype(A), length(A.nzind) + 1)
122-
@inline colstartind(A::SparseMatrixCSC, j) = A.colptr[j]
123-
@inline colboundind(A::SparseMatrixCSC, j) = A.colptr[j + 1]
124-
@inline storedinds(A::SparseVector) = A.nzind
125-
@inline storedinds(A::SparseMatrixCSC) = A.rowval
126-
@inline storedvals(A::SparseVecOrMat) = A.nzval
121+
@inline colboundind(A::SparseVector, j) = convert(indtype(A), length(nonzeroinds(A)) + 1)
122+
@inline colstartind(A::SparseMatrixCSC, j) = getcolptr(A)[j]
123+
@inline colboundind(A::SparseMatrixCSC, j) = getcolptr(A)[j + 1]
124+
@inline storedinds(A::SparseVector) = nonzeroinds(A)
125+
@inline storedinds(A::SparseMatrixCSC) = rowvals(A)
126+
@inline storedvals(A::SparseVecOrMat) = nonzeros(A)
127127
@inline setcolptr!(A::SparseVector, j, val) = val
128-
@inline setcolptr!(A::SparseMatrixCSC, j, val) = A.colptr[j] = val
128+
@inline setcolptr!(A::SparseMatrixCSC, j, val) = getcolptr(A)[j] = val
129129
function trimstorage!(A::SparseVecOrMat, maxstored)
130130
resize!(storedinds(A), maxstored)
131131
resize!(storedvals(A), maxstored)
@@ -214,9 +214,9 @@ end
214214
@inline _all_args_isa(t::Tuple{Broadcasted,Vararg{Any}}, ::Type{T}) where T = _all_args_isa(t[1].args, T) & _all_args_isa(tail(t), T)
215215
@inline _densennz(shape::NTuple{1}) = shape[1]
216216
@inline _densennz(shape::NTuple{2}) = shape[1] * shape[2]
217-
_maxnnzfrom(shape::NTuple{1}, A) = nnz(A) * div(shape[1], A.n)
218-
_maxnnzfrom(shape::NTuple{2}, A::SparseVector) = nnz(A) * div(shape[1], A.n) * shape[2]
219-
_maxnnzfrom(shape::NTuple{2}, A::SparseMatrixCSC) = nnz(A) * div(shape[1], A.m) * div(shape[2], A.n)
217+
_maxnnzfrom(shape::NTuple{1}, A::SparseVector) = nnz(A) * div(shape[1], length(A))
218+
_maxnnzfrom(shape::NTuple{2}, A::SparseVector) = nnz(A) * div(shape[1], length(A)) * shape[2]
219+
_maxnnzfrom(shape::NTuple{2}, A::SparseMatrixCSC) = nnz(A) * div(shape[1], size(A, 1)) * div(shape[2], size(A, 2))
220220
@inline _maxnnzfrom_each(shape, ::Tuple{}) = ()
221221
@inline _maxnnzfrom_each(shape, As) = (_maxnnzfrom(shape, first(As)), _maxnnzfrom_each(shape, tail(As))...)
222222
@inline _unchecked_maxnnzbcres(shape, As::Tuple) = min(_densennz(shape), sum(_maxnnzfrom_each(shape, As)))
@@ -277,18 +277,18 @@ function _map_notzeropres!(f::Tf, fillvalue, C::SparseVecOrMat, A::SparseVecOrMa
277277
end
278278
# helper functions for these methods and some of those below
279279
@inline _densecoloffsets(A::SparseVector) = 0
280-
@inline _densecoloffsets(A::SparseMatrixCSC) = 0:A.m:(A.m*(A.n - 1))
280+
@inline _densecoloffsets(A::SparseMatrixCSC) = 0:size(A, 1):(size(A, 1)*(size(A, 2) - 1))
281281
function _densestructure!(A::SparseVector)
282-
expandstorage!(A, A.n)
283-
copyto!(A.nzind, 1:A.n)
282+
expandstorage!(A, length(A))
283+
copyto!(nonzeroinds(A), 1:length(A))
284284
return A
285285
end
286286
function _densestructure!(A::SparseMatrixCSC)
287-
nnzA = A.m * A.n
287+
nnzA = size(A, 1) * size(A, 2)
288288
expandstorage!(A, nnzA)
289-
copyto!(A.colptr, 1:A.m:(nnzA + 1))
289+
copyto!(getcolptr(A), 1:size(A, 1):(nnzA + 1))
290290
for k in _densecoloffsets(A)
291-
copyto!(A.rowval, k + 1, 1:A.m)
291+
copyto!(rowvals(A), k + 1, 1:size(A, 1))
292292
end
293293
return A
294294
end
@@ -405,7 +405,7 @@ function _map_notzeropres!(f::Tf, fillvalue, C::SparseVecOrMat, As::Vararg{Spars
405405
# Populate values
406406
fill!(storedvals(C), fillvalue)
407407
# NOTE: Combining this fill! into the loop below to avoid multiple sweeps over /
408-
# nonsequential access of C.nzval does not appear to improve performance.
408+
# nonsequential access of nonzeros(C) does not appear to improve performance.
409409
rowsentinel = numrows(C) + 1
410410
stopks = _colstartind_all(1, As)
411411
@inbounds for (j, jo) in zip(columns(C), _densecoloffsets(C))
@@ -812,7 +812,7 @@ function _broadcast_notzeropres!(f::Tf, fillvalue, C::SparseVecOrMat, A::SparseV
812812
return C
813813
end
814814
_finishempty!(C::SparseVector) = C
815-
_finishempty!(C::SparseMatrixCSC) = (fill!(C.colptr, 1); C)
815+
_finishempty!(C::SparseMatrixCSC) = (fill!(getcolptr(C), 1); C)
816816

817817
# special case - vector outer product
818818
_copy(f::typeof(*), x::SparseVectorUnion, y::AdjOrTransSparseVectorUnion) = _outer(x, y)

0 commit comments

Comments
 (0)