Skip to content

map and mapreduce #146

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

Merged
merged 9 commits into from
Jul 4, 2021
Merged

map and mapreduce #146

merged 9 commits into from
Jul 4, 2021

Conversation

mcabbott
Copy link
Contributor

@mcabbott mcabbott commented May 15, 2021

This should make map(f, A, B, C), and mapreduce(f, op, A, B, C) work for FillArrays.

One motivation is that Base's mapreduce isn't fast for multiple arrays, but if we peel off the ones which are Fill then we can go the fast path. The other is that a mix of CuArrays & Fill seems to give errors:

julia> mapreduce((x,y) -> x*conj(y), +, Fill(1.0, 3), cu(ones(3)))
ERROR: scalar getindex is disallowed

Edit -- scope has crept a little, it now also handles reduce, which has this result:

julia> sum(Fill(3,4,5), dims=1)
1×5 Matrix{Int64}:  # master
 12  12  12  12  12

1×5 Fill{Int64}: entries equal to 12  # this PR

Happy to remove / separate that if you prefer.

@mcabbott mcabbott closed this May 16, 2021
@mcabbott mcabbott reopened this May 16, 2021
@mcabbott
Copy link
Contributor Author

Error on 1.0 was real, "UndefVarError: _InitialValue not defined".

I've just disabled these methods on old versions, rather than try to figure out how 1.0 did this & exactly when the change happened. Without them you will more often get an Array, as you do without this PR.

@codecov
Copy link

codecov bot commented May 20, 2021

Codecov Report

Merging #146 (0b68d40) into master (a7ce2f8) will increase coverage by 0.31%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #146      +/-   ##
==========================================
+ Coverage   95.66%   95.97%   +0.31%     
==========================================
  Files           4        4              
  Lines         531      572      +41     
==========================================
+ Hits          508      549      +41     
  Misses         23       23              
Impacted Files Coverage Δ
src/FillArrays.jl 94.27% <ø> (ø)
src/fillbroadcast.jl 97.45% <100.00%> (+1.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7ce2f8...0b68d40. Read the comment docs.

@dlfivefifty
Copy link
Member

Tests are failing on 1.0

@mcabbott
Copy link
Contributor Author

Oh right, sorry, looks like vararg mapreduce didn't exist at all then. Will omit those bits.

@mcabbott
Copy link
Contributor Author

mcabbott commented Jul 4, 2021

Bump?

@dlfivefifty
Copy link
Member

Great if you bump the version number I can tag a new release

@mcabbott
Copy link
Contributor Author

mcabbott commented Jul 4, 2021

Can do, or else #144 has a version bump, too.

@dlfivefifty dlfivefifty merged commit 76ab425 into JuliaArrays:master Jul 4, 2021
@mcabbott mcabbott deleted the mapreduce branch July 4, 2021 13:49
@mcabbott mcabbott mentioned this pull request Jul 18, 2022
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.

2 participants