JuliaFolds / Transducers.jl

Efficient transducers for Julia
https://juliafolds.github.io/Transducers.jl/dev/
MIT License
432 stars 24 forks source link

Transducers improperly promotes elements on reduction #524

Open ExpandingMan opened 2 years ago

ExpandingMan commented 2 years ago
function mwe()
    v = ([1.0, 2.0], [3, 4])
    1:2 |> Map(i -> v[i]) |> collect
end

gives

2-element Vector{Vector{Float64}}:
 [1.0, 2.0]
 [3.0, 4.0]

For whatever reason it seems to want to promote the second element.

ExpandingMan commented 2 years ago

A perhaps more explicit example:

◖◗ x = ([1, 2], [1.0, 2.0]);

◖◗ collect(x)
2-element Vector{Vector}:
 [1, 2]
 [1.0, 2.0]

◖◗ x |> Map(identity) |> collect
2-element Vector{Vector{Float64}}:
 [1.0, 2.0]
 [1.0, 2.0]

Apparently everything based on Transducers including ThreadsX is promoting its elements. Not sure how long it's been going on or how I didn't notice it before. It's a bit scary, if you don't want this behavior your options for threads are reduced to Base.

eliascarv commented 2 years ago

I'm having the same problem implementing a new feature of TableTransforms.jl: JuliaML/TableTransforms.jl#96. Is this the expected behavior of Map @tkf?

ExpandingMan commented 2 years ago

FYI you can work around this by wrapping arguments in Ref, but I think because of the discrepancy with Base.collect this should be considered a bug.

tkf commented 2 years ago

Yeah, the real issue is https://github.com/JuliaFolds/BangBang.jl/issues/230 and the non-Base-compat behaviors of collect and friends are independent of the transducers like Map. The fundamental issue is that various Base functions have rather too context-dependent and undocumented behavior around promotion. However, since Base functions are the outcome of years of improvements for what Julia programmers feel "natural," I think we need to find a good approximation in JuliaFolds.

Meanwhile, it's not difficult to define your own tcollect-like function

julia> function conservative_append!!(xs, ys)
           if eltype(ys) <: eltype(xs)
               append!(xs, ys)
               xs
           else
               zs = similar(
                   xs,
                   Base.promote_typejoin(eltype(xs), eltype(ys)),
                   length(xs) + length(ys),
               )
               copyto!(view(zs, 1:length(xs)), xs)
               copyto!(view(zs, length(xs)+1:length(zs)), ys)
               zs
           end
       end;

julia> using MicroCollections: EmptyVector, SingletonVector

julia> my_collect(xs) =
           xs |>
           Map(x -> SingletonVector((x,))) |>
           foldxt(conservative_append!!; init = EmptyVector());

(This is not OffsetArray-correct but it's straightforward to support non-base-1 vectors)

This produces a result similar to Base.collect

julia> function mwe()
           v = ([1.0, 2.0], [3, 4])
           1:2 |> Map(i -> v[i]) |> my_collect
       end;

julia> mwe()
2-element Vector{Vector}:
 [1.0, 2.0]
 [3, 4]

julia> let v = ([1.0, 2.0], [3, 4])
           collect(v[i] for i in 1:2)
       end
2-element Vector{Vector}:
 [1.0, 2.0]
 [3, 4]

Perhaps a smaller MWE is

julia> tcollect(x for x in Any[1, 2.0])  # aggressive promotion (similar to `vcat` and broadcasting)
2-element Vector{Float64}:
 1.0
 2.0

julia> my_collect(x for x in Any[1, 2.0])  # conservative promotion (similar to `collect`)
2-element Vector{Real}:
 1
 2.0

julia> collect(x for x in Any[1, 2.0])
2-element Vector{Real}:
 1
 2.0