-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Deprecate vectorized abs2 methods in favor of compact broadcast syntax #18564
Conversation
@@ -1,4 +1,4 @@ | |||
# This file is a part of Julia. License is MIT: http://julialang.org/license | |||
9# This file is a part of Julia. License is MIT: http://julialang.org/license |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gremlins. Fixed. Thanks!
Rebased. Best! |
your sparse vector broadcast pr will fix the fusing concern here, right? |
Should, yes. Thanks! |
@@ -919,7 +919,8 @@ hvcat{T}(rows::Tuple{Vararg{Int}}, xs::_TypedDenseConcatGroup{T}...) = Base.type | |||
|
|||
# zero-preserving functions (z->z, nz->nz) | |||
broadcast(::typeof(abs), x::AbstractSparseVector) = SparseVector(length(x), copy(nonzeroinds(x)), abs.(nonzeros(x))) | |||
for op in [:abs2, :conj] | |||
broadcast(::typeof(abs2), x::AbstractSparseVector) = SparseVector(length(x), copy(nonzeroinds(x)), abs2.(nonzeros(x))) | |||
for op in [:conj] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will conj be implemented as a method of broadcast once that supports sparse vectors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC conj
methods for vectors/matrices should persist per discussion in #18495 (comment). Thoughts? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right the method will stay without dots, I'm asking if this implementation will remain. not really a feature freeze issue, but possible cleanup for later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers, with you now. Definitely a candidate for cleanup, and likewise for several more methods in base/sparse/sparsevectors.jl. Best!
Subsumed by #19791. |
This PR deprecates the last remaining vectorized
abs2
method in favor of compact broadcast syntax. Ref. #16285, #17302, #18495, #18512, #18513, and #18558. Best!(Unlike with
float
,real
, etc., the remaining vectorizedabs2
method never aliases its input. This PR should be less controversial than #18495, #18512, and #18513 as a result.)