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

Make reduce(vcat, vecs) work on a tuple of arrays #54046

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mcabbott
Copy link
Contributor

@mcabbott mcabbott commented Apr 11, 2024

The fast method reduce(vcat, [v, w, x, y, z]) exists to avoid splatting the outer vector to vcat(v, w, x, y, z). But it seems fairly common to assume this exists for tuples too, where there is no splat penalty, but reduce(vcat, (v, w, x, y, z)) in fact works pairwise, vcat(vcat(vcat(vcat(v, w), x), y), z). (For example, today on discourse.) This PR wants to route it to the fast code, which turns out to work equally well:

julia> let vs = [rand(10) for _ in 1:10]
          tvs = Tuple(vs)
          @btime reduce(vcat, $vs)
          @btime reduce(vcat, $tvs)  # before, this is slow
       end;
  min 93.399 ns, mean 138.863 ns (4 allocations, 1.05 KiB)
  min 317.416 ns, mean 547.738 ns (18 allocations, 5.00 KiB)

julia> using Revise; Revise.track(Base)  # load this PR

julia> let vs = [rand(10) for _ in 1:10]
          tvs = Tuple(vs)
          @btime reduce(vcat, $vs)
          @btime reduce(vcat, $tvs)  # after, this is as fast as vcat
          @btime vcat($tvs...)
       end;
  min 94.839 ns, mean 141.891 ns (4 allocations, 1.05 KiB)
  min 75.719 ns, mean 110.381 ns (2 allocations, 928 bytes)
  min 74.606 ns, mean 109.603 ns (2 allocations, 928 bytes)

The PR does the same for reduce(hcat, xs). For a tuple of vectors this agrees with stack, but a tuple of matrices will be different. One question here is whether it should skip the one-element case, as the code in the first commit changes this return type (Edit: xref #31169 and #34380 about this):

julia> reduce(hcat, ([1],))  # before, hcat is not called
1-element Vector{Int64}:
 1
 
julia> reduce(hcat, ([1],))  # after, it is called once
1×1 Matrix{Int64}:
 1

Another question is whether these should apply to NamedTuples too, as those show up in the wild too, and would be easy to send to the fast routine.

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

Successfully merging this pull request may close these issues.

None yet

1 participant