Deprecate vectorized float methods in favor of compact broadcast syntax

[Sacha0]

This PR deprecates all (?) remaining vectorized float methods in favor of compact broadcast syntax. Ref. #16285 and #17302. Best!

To work around #18462, I made the Broadcast module import rather than export broadcast. I will correct this if / when a better solution for #18462 appears.

[stevengj]

Probably we should also have broadcast{T<:AbstractFloat}(::typeof(float), A::AbstractArray{Complex{T}}) = A in complex.jl

[Sacha0]

Added. Thanks!

[Sacha0]

CI appears happy now, so I've nixed the WIP tag / this should be ready for review. Best!

[simonster]

I'm not sure how I feel about broadcast returning the input rather than a copy. Has this already been discussed somewhere?

[Sacha0]

I'm not sure how I feel about broadcast returning the input rather than a copy. Has this already been discussed somewhere?

Likewise, and not that I am aware of (though I have not searched). The new broadcast methods are direct translations of (and hence drop-in replacements for) the former float methods. Seamless initial transition, additional changes later was my thinking. Thoughts?

[stevengj]

In all the cases where we use float(array), we don't want it to make a copy if the array is already floating-point; it makes some sense to do the same thing for float.(array) as a specialized broadcast method. If you compose this with another operation, e.g. sin.(float.(array)) then the loops are fused, the specialized method is not called, and a new array is returned.

[stevengj]

Somewhat perversely, however, the loop fusion means that float.(float.(array)) will make a copy. This doesn't seem like a practical concern to me.

[Sacha0]

In all the cases where we use float(array), we don't want it to make a copy if the array is already floating-point; it makes some sense to do the same thing for float.(array) as a specialized broadcast method.

Agreed. On the other hand, having float.(array) alias array implies that broadcast's (presently implicit, hopefully one day explicit) contract does not guarantee absence of aliasing (a subject which probably evokes strong sentiments :) ).

[simonster]

I'm not totally sure that these should be broadcast methods for the reason mentioned by @Sacha0 above, but if they are, I think we should specifically document that these particular methods might return the input array to avoid various forms of madness.

[stevengj]

We could put it in the float docstring.

[Sacha0]

The more I think it through, the more I err on the side of broadcast not providing any aliasing guarantees (similar to convert, and for similar reasons). With broadcast's expanding role and increasing sophistication, providing such guarantees may come at a steep price (for example broadcast not being able to unify/replace the hodgepodge of vectorized methods as was intended). And one can always ensure broadcast generates a result that does not alias the input (and without introducing unnecessary copies) by forcing fusion, for example by injecting an identity into the call as @stevengj points out in #18495 (comment).

[simonster]

I am strongly against broadcast making no aliasing guarantees whatsoever. It's great if you are trying to optimize code but dangerous if you are trying to write it. If we made no guarantees at all, then identity would not be sufficient to guarantee that a new array is returned, because we could decide not to later. abs.(x::Vector{UInt}) and real.(x::Vector{Complex128}) return a new array now, but abs could return the same input and real could return a view into the reinterpreted vector. We could optimize out X .* 1 if we wanted to. People will write a lot of code that relies on unspecified behavior. It's much harder to reason about than convert, where there is a guarantee that converting an object to a different type will return a different object.

I am more undecided about whether broadcast should guarantee to return a new array for most functions, but not for a select few other functions. I guess the question is, does broadcast mean "do something to an array" (more like convert) or "do something to the elements of an array" (more like map). In the former case, I can understand how the plan here makes sense. In the latter case, I'm less convinced. A function that returns its input is really operating on the input, and not the elements of the input. I'm leaning toward the latter position, because I think that having unary map and broadcast do different things is confusing, and I don't really see a huge downside to leaving these functions as they are.

[simonster]

Also, unless this makes it into 0.5.0, it is technically breaking (unless we decide that code that relied on broadcast returning a new array was always broken) but probably won't break anything.

[Sacha0]

If we made no guarantees at all, then identity would not be sufficient to guarantee that a new array is returned, because we could decide not to later.

Good point! My thinking was specious. The fusion mechanism's behavior is (and probably should remain) unspecified, so the trick I proposed is precarious.

People will write a lot of code that relies on unspecified behavior. It's much harder to reason about than convert, where there is a guarantee that converting an object to a different type will return a different object.

These statements further underscore your point (though perhaps inadvertently): Note that converting an object to a different type may result in aliasing. Examples: converting to a Nullable potentially results in aliasing. converting between AbstractSparseArray subtypes potentially results in aliasing. Prior to #17723 (0.6), converting to AbstractTriangular subtypes potentially resulted in aliasing. These are only the examples I am immediately familiar with; I imagine others exist.

I am more undecided about whether broadcast should guarantee to return a new array for most functions, but not for a select few other functions.

I would advocate against broadcast's contract making a guarantee followed by a list of exceptions. As our exchange above evidences, wandering astray even of simple contracts is easy.

I don't really see a huge downside to leaving these functions as they are.

My primary concern was performance, particularly what @stevengj points out in #18495 (comment). That is, vectorized methods that no-op for particular types are common. Replacing those methods with broadcast methods that necessarily copy might impact performance. As you point out,

making no aliasing guarantees whatsoever. It's great if you are trying to optimize code

and leaves the door open for more exotic fusion-like behaviors, for example those discussed in #16285 (potentially involving intermediary lazy maps / broadcasts / reductions).

But after some more thinking, perhaps that concern was specious too:

(edit: <skip for better flow>)Consider a vectorized float call. If the caller strictly expected A <: AbstractArray{Union{<:AbstractFloat,Complex{<:AbstractFloat}}}, the caller would (should) have omitted the float call (and asserted the type expectation up stream). Hence it seems reasonable to assume that the caller does not strictly expect A <: AbstractArray{Union{<:AbstractFloat,Complex{<:AbstractFloat}}}, in which case the caller should not rely on the corresponding float method's no-op behavior. So regarding breakage / correctness, uniformly replacing float(A) with non-aliasing float.(A) seems reasonably safe. (edit: </skip for better flow>)

Regarding performance, suppose a vectorized float call appears nested in other (return-allocating) vectorized function calls (e.g. sin(float(A))). Even with float a no-op, the caller should expect allocation of at least one new object. Thanks to fusion, the compact broadcast translation sin.(float.(A)) satisfies that expectation independent of whether float.(A) allocates when called in isolation. Of course performance degrades if something prevents fusion (e.g. an interceding vectorized function call), but hopefully that concern will progressively vanish (e.g with further devectorization).

Now instead suppose the vectorized float call appears without nesting in other (return-allocating) vectorized function calls (e.g. float(A)). Here either (1) the caller knows that float allocates for some argument types and accepts the potential performance impact independent of argument type; or (2) the caller accepts the performance impact of allocation for relevant types, but relies upon the absence of allocation for others. In case (1), replacing float(A) with a uniformly allocating float.(A) is fine.

Thus (2) seems like the only case of (near-term, practical) concern. I audited the usages changed in this PR and could not find an instance where float(A) in isolation was not already allocating.

So how does this sound: For now, replace no-op vectorized methods such as float(A::AbstractArray{Union{<:AbstractFloat,Complex{<:AbstractFloat}}) with broadcast specializations that return a copy. Keep an eye on performance. If the performance impact is significant, revisit the issue. Thoughts? Thanks and best!

[stevengj]

(Actually, the fusion mechanism's behavior is specified in the manual. This is necessary because it affects the semantics of dot calls when there are side effects, so it is guaranteed that fusing always occurs.)

[stevengj]

float(x) is used all over Base and packages with the intention that no copy is made for arrays that are already floating point. I don't think it is acceptable to eliminate this functionality.

I don't think it is unreasonable to simply document (in the float docstring) that float.(x) by itself returns x if x is already floating point, but if the float.(x) call is fused with other nested dot calls then the fused loop will still return a new array.

[simonster]

As long as it's fully specified and documented, I don't think it's too bad to make float.(x) return its input, but if we didn't already have a float(x) function that we saw a possibility to replace, I'm not sure we'd choose this behavior.

What's the reason not to leave float(x) as is? Given that it converts Complex{Int} to Complex{Float64}, it doesn't seem crazy that it also converts Array{Int} to Array{Float64}.

[stevengj]

The argument for not leaving it as-is is that we're training users to use f.(x) for elementwise operations, and if we succeed they not likely to discover that there are "random" functions like float(x) that still have traditional "vectorized" methods.

[Sacha0]

As @pabloferz notes in #18618 (comment),

maybe the problem is that people find the new dot syntax too convenient.

The new syntax is fantastic, resulting in a drive to use it for everything. In this case we are trying to pack all existing vectorized methods into that new syntax. Most existing vectorized methods exhibit strict broadcast semantics, no issue there. But a few exhibit convert semantics (namely float, big, and complex, the documentation for each of which begins Convert ...). And a few others mix convert and broadcast semantics (namely real and conj).

Perhaps float, big, and complex should be handled as follows: Give their vectorized methods' behaviors to convert rather than broadcast. Deprecate those vectorized methods with the depwarn pointing to both convert and broadcast, prompting the user to disambiguate her intention when migrating code. This approach avoids weakening broadcast's contract, provides replacements for both present uses of float, and improves consistency overall.

real and conj are trickier. Before going there, thoughts on the preceding proposal for float, big, and complex? Thanks and best!

[simonster]

The observation that some of these functions really have convert semantics is quite useful. Unfortunately, convert doesn't really seem like it can replace these functions, because Julia's type parameters are invariant. Ignoring its behavior for Complex, float means "convert an array of reals to an array of floating point numbers." There's no way to use convert to satisfy this. The obvious option, convert(Array{FloatingPoint}, x), should return an Array{FloatingPoint}, not an Array{Float64}. In #8974 language:

float(x) = convert(@UnionAll(T<:Union{FloatingPoint,UnionAll(S<:FloatingPoint,Complex{S})},Array{T}), x)
big(x) = convert(Union{Array{BigInt},Array{BigFloat}}, x)
complex(x) = convert(@UnionAll(T<:Complex,Array{T}), x)

big seems like a candidate for replacement with convert since you will usually know if you want an Array{BigInt} or an Array{BigFloat}. The others seem harder. Even if we could write the syntax above, it's really far too verbose.

[stevengj]

I wonder if the dot fusion should pass a tuple of the functions that were fused, which might help to propagate properties like zero-preservation in a type-stable way. But it feels too complicated.

The thing is, in principle just checking f(0...) == 0 && return sparsearray should be possible to express in a type-stable way, if we can tell the compiler somehow to assume that the result of this test only depends on the type of f.

[Sacha0]

Unfortunately, convert doesn't really seem like it can replace these functions, because Julia's type parameters are invariant.

Good point! Invariance. I had convert(AbstractArray{AbstractFloat}, A) in mind, but as you say that won't fly.

[Sacha0]

@stevengj, did you mean to post #18495 (comment) in #18590? Best!

[stevengj]

Whoops, right.

[StefanKarpinski]

Bump?

[Sacha0]

Re. #18495 (comment), why not convert(Array{float(eltype(A))}, A)? Perhaps still a bit verbose, but not too bad? Best!

[stevengj]

I would leave real and conj and imag as-is, because it is perfectly standard linear algebra to take e.g. the complex-conjugate of a vector or matrix. These are not just "element-wise" operations; from an algebraic perspective one would expect to have them defined for arrays (in the same way that we still define +, although it is functionally equivalent to .+).

[Sacha0]

I would leave real and conj and imag as-is, because it is perfectly standard linear algebra to take e.g. the complex-conjugate of a vector or matrix. These are not just "element-wise" operations; from an algebraic perspective one would expect to have them defined for arrays (in the same way that we still define +, although it is functionally equivalent to .+).

This argument for preserving real, imag, and conj for non-scalars (and the beautiful related argument for zero/iszero in #17623) is compelling. Absent objections (?) I will close the deprecation pull requests for real, imag, and conj.

It seems consensus favors deprecating vectorized big (?), leaving only vectorized float at issue? Thanks and best!

[Sacha0]

Writing e.g. Array{float(eltype(A))}(A) for convert semantics doesn't strike me as onerous, or at least not so onerous that it would justify float and complex remaining as the only vectorized functions in the language (and with inconsistent aliasing behavior). Thoughts? Thanks!

[stevengj]

Array{float(eltype(A))}(A) seems awfully onerous and error-prone to me. I'd rather leave float(A) and complex(A) as-is for now.

[Sacha0]

(Ref. #22236 (comment).)