Skip to content
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

Transducers improperly promotes elements on reduction #524

Open
ExpandingMan opened this issue Jun 7, 2022 · 4 comments
Open

Transducers improperly promotes elements on reduction #524

ExpandingMan opened this issue Jun 7, 2022 · 4 comments

Comments

@ExpandingMan
Copy link

ExpandingMan commented Jun 7, 2022

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 ExpandingMan changed the title bizarre inference bug in `collect bizarre inference bug in collect Jun 7, 2022
@ExpandingMan
Copy link
Author

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.

@ExpandingMan ExpandingMan changed the title bizarre inference bug in collect Transducers improperly promotes elements on reduction Jun 8, 2022
@eliascarv
Copy link

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
Copy link
Author

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
Copy link
Member

tkf commented Jun 13, 2022

Yeah, the real issue is JuliaFolds/BangBang.jl#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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants