Skip to content

Commit

Permalink
Fix initial value handling with flatten transducer (#34369)
Browse files Browse the repository at this point in the history
There was a bug in initial value handling of `FlatteningRF` and the
following example failed:

    @test mapfoldl(
        x -> (x, x),
        ((a, b), (c, d)) -> (min(a, c), max(b, d)),
        Iterators.flatten((1:2, 3:4)),
    ) == (1, 4)

This is because `BottomRF(op.rf)` was called inside `FlatteningRF`
where `op.rf` is already a "non-bottom" reducing function; here it's a
`MappingRF`.  As `BottomRF(rf)` forwards anything on the second
argument on the first invocation as the first argument (accumulator)
of the next calls, we need to make sure that this value is processed
through `MappingRF` in the above example.  However, if we do
`BottomRF(op.rf)` where `op.rf` is a `MappingRF`, this `BottomRF`
bypasses any processing that has to happen in `op.rf`.

(cherry picked from commit 0ee3264)
  • Loading branch information
tkf authored and KristofferC committed Jan 15, 2020
1 parent a44f86f commit f8fc5fa
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
2 changes: 1 addition & 1 deletion base/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ struct FlatteningRF{T}
end

@inline function (op::FlatteningRF)(acc, x)
op′, itr′ = _xfadjoint(BottomRF(op.rf), x)
op′, itr′ = _xfadjoint(op.rf, x)
return _foldl_impl(op′, acc, itr′)
end

Expand Down
8 changes: 8 additions & 0 deletions test/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -535,3 +535,11 @@ x = [j^2 for j in i]
i = Base.Slice(0:0)
x = [j+7 for j in i]
@test sum(x) == 7

@testset "initial value handling with flatten" begin
@test mapfoldl(
x -> (x, x),
((a, b), (c, d)) -> (min(a, c), max(b, d)),
Iterators.flatten((1:2, 3:4)),
) == (1, 4)
end

0 comments on commit f8fc5fa

Please sign in to comment.