Deprecate vectorized two-argument complex in favor of compact broadcast syntax #19712
Conversation
| @@ -512,8 +512,6 @@ function copy!(S::SharedArray, R::SharedArray) | |||
| return S | |||
| end | |||
|
|
|||
| complex(S1::SharedArray,S2::SharedArray) = convert(SharedArray, complex(S1.s, S2.s)) | |||
There was a problem hiding this comment.
complex.(S1, S2) would be at the mercy of similars return type here, right? so would not return a SharedArray any more unless we were to go back on similar returning SharedArray ?
There was a problem hiding this comment.
Would it make sense to define a separate broadcast method, like
broadcast{T<:SharedArray}(::typeof(complex), S1::T, S2::T) = convert(T, complex.(S1.s, S2.s))?
There was a problem hiding this comment.
complex.(S1, S2) would be at the mercy of similars return type here, right? so would not return a SharedArray any more unless we were to go back on similar returning SharedArray ?
IIUC, yes, complex.(A, B) for SharedArrays A and B would yield an appropriate <:Array rather than <:SharedArray. Best to keep this method as a broadcast specialization as @ararslan suggests, or instead require the more explicit convert(SharedArray, complex.(A, B)) going forward? (Fused cases will require the latter in any case unless we build a generic broadcast for SharedArrays.) Thanks!
There was a problem hiding this comment.
I would not recommend defining broadcast(::typeof(complex), ...), because it will not be called in the presence of fusion etcetera.
If you want a SharedArray output, one option is to use in-place assignment (mysharedarray .= complex.(a,b)). Another possibility would to define Base.Broadcast.containertype promotion methods to always produce a SharedArray from broadcast on SharedArray inputs, if that is the desired behavior.
|
Before I spend more time on this pull request: @stevengj, are you in favor of this change (as opposed to deprecating single-argument |
|
Yes, for the two-argument |
… broadcast syntax.
Sacha0
force-pushed
the
devectwoargcomplex
branch
from
cd0f380 to
997785b
Compare
|
Revised extensively. |
This pull request deprecates most remaining two-argument
complexmethods (less a few defined in base/sparse/sparsevectors.jl that depend on #19690) in favor of compactbroadcastsyntax. Best!(Two-argument vectorized
complexmethods always return a fresh array. So in contrast to one-argument vectorizedcomplexmethods (which suffer from the same controversy asfloat, namely what to do about ad hoc aliasing semantics as in #18495), these deprecations should be uncontroversial.)