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

Move broadcast to iteration method #226

Merged
merged 11 commits into from
Mar 26, 2019
Merged

Move broadcast to iteration method #226

merged 11 commits into from
Mar 26, 2019

Conversation

piever
Copy link
Collaborator

@piever piever commented Mar 26, 2019

This simplifies broadcast a bit and avoids relying on type inference for the result. As usual with this translation, I need to look at performance a bit before merging.

@piever piever changed the title Move broadcast to iteration method WIP: Move broadcast to iteration method Mar 26, 2019
@piever piever mentioned this pull request Mar 26, 2019
12 tasks
@piever
Copy link
Collaborator Author

piever commented Mar 26, 2019

Performance is fixed now:

using IndexedTables, SparseArrays, BenchmarkTools, Random
Random.seed!(666)
idx = Columns(p=rand(1:100, N), q=rand(1:100, N))
t = NDSparse(idx, rand(N))
t2 = NDSparse(Columns(q=rand(1:100, N)), rand(N))

@btime broadcast(+, $t, $t2)
# master 296.955 μs (200 allocations: 941.16 KiB)
# PR 289.171 μs (741 allocations: 898.08 KiB)

Random.seed!(666)
S = sprand(1000, 1000,.1)
v = rand(1000)
nd = convert(NDSparse, S)
ndv = convert(NDSparse,v)
@btime broadcast(*, $nd, $ndv)
#master 6.247 ms (113 allocations: 6.01 MiB)
#PR 6.421 ms (6119 allocations: 7.01 MiB)

I'm leaving the file with the benchmarks I was using for join and broadcast in the benchmarks folder so we can go back to it if we want to optimize further.

@piever piever changed the title WIP: Move broadcast to iteration method Move broadcast to iteration method Mar 26, 2019
@piever piever merged commit c45dc71 into master Mar 26, 2019
@shashi shashi deleted the pv/broadcast branch March 27, 2019 07:23
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.

1 participant