Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SparseArrays: specialize zero(...) so result has nnz(...) = 0 #34209

Conversation

tkluck
Copy link
Contributor

@tkluck tkluck commented Dec 28, 2019

This pull request fixes #31835.

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.

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.
@ViralBShah ViralBShah added the domain:arrays:sparse Sparse arrays label Dec 28, 2019
@tkluck
Copy link
Contributor Author

tkluck commented Dec 28, 2019

Thanks for approving @ViralBShah !

Just had a look at the CI failure. The only failure is buildbot/tester_win32 and nothing in the logs seems related to this change. Should be good to merge.

@ViralBShah ViralBShah merged commit 0570202 into JuliaLang:master Dec 28, 2019
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zero(::AbstractSparseArray) full of structural nonzeros set to zero
2 participants