vectorized float() deprecations

[c42f]

Vectorized float() is currently deprecated only for AbstractArray{<:Complex}, but not for other types of numbers or strings.

This PR deprecates all vectorized versions of float, replacing uses of such with broadcast(), and adding a few broadcast specializations where necessary to maintain the efficiency of a few specialized vectorized versions.

This came up over at JuliaArrays/StaticArrays.jl#247

[andreasnoack]

I think there was a concern about the extra copy when the element type is already floating point like, i.e.

julia> x = randn(1);

julia> float(x) === x
true

as a feature. I'm not sure how much it is used, though. The problem is that converting to a floating point type via convert is cumbersome because the floating point property doesn't fit well into the declared type tree, e.g. floating point types are both <:AbstractFloat and Complex{<:AbstractFloat} but not Complex{<:Integer}.

[c42f]

So, @andreasnoack do you mean we should or should not have this optimization?

At the moment I left it in here to duplicate the old behavior of avoiding the copy. A counter argument might be that people usually expect broadcast to make a copy of the data, and optimizing that away in particular cases is too confusing?

[andreasnoack]

Ah, sorry. I missed that "adding a few broadcast specializations" was exactly about this issue. I'm not sure what my opinion is. Always getting a copy is easy to reason about but avoiding an unnecessary copy is also a good thing.

[c42f]

No problem, I'm not sure what's better here either. At the moment I went for the faster version, as it allows broadcast to be a drop in replacement for vectorized float().

Do we have any precedent for not returning a copy of the data in broadcast()? I had a quick look through Base, and all I found was was that broadcast(::typeof(xor), B::BitArray, x::Bool) takes the opposite stance and calls copy() when it wouldn't otherwise be required.

[tkelman]

these aren't in the right section since these lines are scheduled to be deleted before 0.7 rc's, but newly modified deprecations should stay until 1.0-pre

[Sacha0]

Deprecating the remaining vectorized float methods was discussed at length in #18495. Best!

[c42f]

Oh dear, I hope I'm not the only one who has trouble finding old discussions on github :-/ I thought I'd just jump in and fix this as a small and obvious inconsistency with a PR, rather than opening an issue!

So obviously there's some prior opinion expressed in #18495 that broadcast should have copy semantics. It looks like people gave this a bit of thought - it would be great if that thought had made it into the source code as a rationale for the way float currently is.

There's also evidence that code is now already relying on broadcast to create a copy: the main test failures here are in linalg/dense.jl - after some digging, it's because of a call powm!(UpperTriangular(float.(A)), with A subsequently used again.

So I think on the whole, that I'm going to back away from this specific solution slowly with arms where you can see them ;-)

However, there's a few issues unaddressed:

• Why is vectorized float(::AbstractArray{<:Complex}) deprecated, and was that intentional? @Sacha0 perhaps you could comment on that (cf edb235a) ?
• If float() for non-float arrays were implemented with map() or broadcast(), I could just let Base do the work, and not have to reimplement it in StaticArrays. Presumably an easy and non-controversial change.
• The documentation doesn't discuss the semantics of the vectorized version.

I can update this PR to be more minimal and just address these few issues (unless anybody thinks I should make a separate PR for that).

[Sacha0]

Oh dear, I hope I'm not the only one who has trouble finding old discussions on github :-/

You are not the only one I assure you! :)

Why is vectorized float(::AbstractArray{<:Complex}) deprecated, and was that intentional? @Sacha0 perhaps you could comment on that (cf edb235a) ?

Good question! My recollection is hazy. But IIRC, those methods were vectorized via @vectorize_?arg. @vectorize_?arg-vectorized methods were necessarily copying. So deprecating those methods in favor of broadcast (which came before the vectorized float-and-friends aliasing question was realized) was uncontroversial.

(FWIW, I would support deprecating vectorized float in favor of broadcast [without specializations that alias] and convert [for where potential aliasing was intended / meaningfully improves performance, which appeared to be a small minority of cases].)

Best!

[TotalVerb]

convert(AbstractArray{<:AbstractFloat}, x) seems like a very reasonable idiom for vectorized float(x), and convert(AbstractFloat, x) a very reasonable idiom for non-vectorized float(x).

[Sacha0]

convert(AbstractArray{<:AbstractFloat}, x) seems like a very reasonable idiom for vectorized float(x)

Unfortunately that and similar incantations won't work IIRC; see the discussion beginning #18495 (comment). Instead incantations like convert(Array{float(eltype(A))}, A) are necessary. In the relatively few cases where convert semantics are necessary, the typing overhead doesn't strike me as onerous relative to maintaining this exception in the language. But opinions differ :).

[TotalVerb]

Note the AbstractArray{<:AbstractFloat}. It certainly does express the operation fine.

[TotalVerb]

To be clear, I'm not saying this works right now; but there is no reason we can't specialize that particular convert to do what float does.

[Sacha0]

Yes, sorry I missed the UnionAll there :).

[TotalVerb]

What about the following deprecations:

float(x::Real) to convert(AbstractFloat, x) (user can use AbstractFloat(x))
float(x::Complex) to convert(Complex{<:AbstractFloat}, x) (user can use Complex{<:AbstractFloat}(x))
float(x::AbstractArray{<:Real}) to convert(AbstractArray{<:AbstractFloat}, x)
float(x::AbstractArray{<:Complex}) to convert(AbstractArray{<:Complex{<:AbstractFloat}}, x)

[JeffBezanson]

Looks like we are not doing this.