-
-
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 div methods in favor of compact broadcast syntax #18607
Conversation
Ping ... you have a bunch of PRs like this. What is their status? Are they waiting on the sparse-broadcast stuff to stabilize? |
Echo :). Yes, I thought these pull requests were blocked on (at least) completion of the sparse broadcast redesign/rewrite. If I mistook earlier discussion and you feel these pull requests should move forward immediately, please let me know. Otherwise, what mark do you feel sparse/structured broadcast functionality should hit for these pull requests to move forward? Status of the sparse broadcast redesign/rewrite: I plan to rebase and merge #19371 following this post, and likewise #19438 tomorrow. #19438 was a warmup for giving Also if you have a timescale in mind due to other pull requests being blocked on this work, please let me know. Thanks for the ping and best! |
That sounds reasonable. |
I think there is supposed to be a feature freeze at the end of the year, though, so it would good to have the sparse-broadcast stuff and hence the vectorized dot operators by then. |
Agreed, hence my timescale question. That is, I want to make certain I leave you with enough time to e.g. comfortably wrap up #17623 before feature freeze. If that means I should finish the sparse broadcast rewrite in the next week or some such, let me know and I will prioritize it. Best! |
F[i] = div(B[i], x) | ||
end | ||
return F | ||
end |
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.
Did #17623 nix a former version of this method? If so, I will remove this method.
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.
Yup, all the broadcast(::typeof(foo), ...)
methods for BitArray
were nixed.
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, removing this new method and will likewise remove the definition for mod
that remains (just below) in the relevant PR. (I suppose this change also moves div
's promotion behavior in the direction of #19669.) Thanks!
Rebased. Best! |
r1 = func(args...) | ||
r2 = func(map(x->(isa(x, BitArray) ? Array(x) : x), args)...) | ||
check_bitop_tests(ret_type, r1, r2) | ||
end | ||
function check_bitop_dotcall(ret_type, func, args...) |
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.
did the uses of this all get deleted?
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.
Oh, good point! Removing that. 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.
Removed. Thanks!
@@ -66,7 +66,7 @@ promote_array_type{S<:Integer}(::typeof(/), ::Type{S}, ::Type{Bool}, T::Type) = | |||
promote_array_type{S<:Integer}(::typeof(\), ::Type{S}, ::Type{Bool}, T::Type) = T | |||
promote_array_type{S<:Integer}(F, ::Type{S}, ::Type{Bool}, T::Type) = T | |||
|
|||
for f in (:+, :-, :div, :mod, :&, :|, :xor) | |||
for f in (:+, :-, :mod, :&, :|, :xor) |
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.
(This change moves div
's promotion behavior in the direction of #19669.)
@@ -89,7 +89,7 @@ function _elementwise{T}(op, ::Type{T}, A::AbstractArray, B::AbstractArray) | |||
return F | |||
end | |||
|
|||
for f in (:div, :mod, :rem, :&, :|, :xor, :/, :\, :*, :+, :-) | |||
for f in (:mod, :rem, :&, :|, :xor, :/, :\, :*, :+, :-) |
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.
(Likewise here, this change moves div
's promotion behavior in the direction of #19669.)
9ee217c
to
340574a
Compare
Subsumed by #19791. |
This PR deprecates (almost) all remaining vectorized
div
methods (less a couple related to dates, separate PR) in favor of compact broadcast syntax. Ref. #16285, #17302, #18495, #18512, #18513, #18558, #18564, #18566, #18571 #18575, #18576, #18586, #18590, and #18593. Best!(Unlike with
float
,real
, etc., the remaining vectorizeddiv
methods never alias their input. This PR should be less controversial than #18495, #18512, and #18513 as a result.)