Skip to content

Commit

Permalink
SparseArrays: specialize zero(...) so result has nnz(...) = 0 (#34209)
Browse files Browse the repository at this point in the history
Before this commit, the result of

    zero(x::AbstractSparseArray)

is filled with stored zeros exactly where `x` has a stored value. That is a
wasteful artifact of the default fallback implementation for `AbstractArray`.
After this commit, we return a sparse array of the same dimensions as `x`
without any stored values.

This change is backwards incompatible where it relates to object identity.
Before this commit, for mutable objects, every stored zero has the same
identity. Example:

    julia> using SparseArrays

    julia> y = zero(BigInt[1,2,3]); y[1] === y[2]
    true

    julia> y = zero(sparse(BigInt[1,2,3])); y[1] === y[2]
    true

    julia> Base.zero(a::AbstractSparseArray) = spzeros(eltype(a), size(a)...)

    julia> y = zero(sparse(BigInt[1,2,3])); y[1] === y[2]
    false

But this behaviour should be classified as a performance bug and can therefore
be fixed in a minor (but not a patch) release.
  • Loading branch information
tkluck authored and ViralBShah committed Dec 28, 2019
1 parent acb7bd9 commit 0570202
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 1 deletion.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ Standard library changes

#### SparseArrays

* The return value of `zero(x::AbstractSparseArray)` has no stored zeros anymore ([#31835]).
Previously, it would have stored zeros wherever `x` had them. This makes the operation
constant time instead of `O(<number of stored values>)`.

#### Dates

#### Statistics
Expand Down
4 changes: 3 additions & 1 deletion stdlib/SparseArrays/src/SparseArrays.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ using Base: ReshapedArray, promote_op, setindex_shape_check, to_shape, tail,
using Base.Sort: Forward
using LinearAlgebra

import Base: +, -, *, \, /, &, |, xor, ==
import Base: +, -, *, \, /, &, |, xor, ==, zero
import LinearAlgebra: mul!, ldiv!, rdiv!, cholesky, adjoint!, diag, eigen, dot,
issymmetric, istril, istriu, lu, tr, transpose!, tril!, triu!,
cond, diagm, factorize, ishermitian, norm, opnorm, lmul!, rmul!, tril, triu, matprod
Expand Down Expand Up @@ -52,6 +52,8 @@ similar(D::Diagonal, ::Type{T}, dims::Union{Dims{1},Dims{2}}) where {T} = spzero
similar(S::SymTridiagonal, ::Type{T}, dims::Union{Dims{1},Dims{2}}) where {T} = spzeros(T, dims...)
similar(M::Tridiagonal, ::Type{T}, dims::Union{Dims{1},Dims{2}}) where {T} = spzeros(T, dims...)

zero(a::AbstractSparseArray) = spzeros(eltype(a), size(a)...)

const BiTriSym = Union{Bidiagonal,SymTridiagonal,Tridiagonal}
function *(A::BiTriSym, B::BiTriSym)
TS = promote_op(matprod, eltype(A), eltype(B))
Expand Down
1 change: 1 addition & 0 deletions stdlib/SparseArrays/test/sparse.jl
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ end
@testset "issparse" begin
@test issparse(sparse(fill(1,5,5)))
@test !issparse(fill(1,5,5))
@test nnz(zero(sparse(fill(1,5,5)))) == 0
end

@testset "iszero specialization for SparseMatrixCSC" begin
Expand Down

4 comments on commit 0570202

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your test job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

Please sign in to comment.