This repository has been archived by the owner on May 4, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 21
Rewrite broadcast() and map() based on lift() #166
Merged
+255
−645
Merged
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
4233744
Rewrite broadcast() using Base implementation and lifting semantics
nalimilan 899ed75
Use generated function to avoid allocation
nalimilan 23ec9f2
Fix performance without generated functions
nalimilan 2953516
Improve efficiency of null checking for generic case
nalimilan 82d9fde
Fix call to null_safe_op() for generic case
nalimilan cc787d9
Fix remaining allocations in generic method
nalimilan 0d649dd
Add non-splatting methods up to 7 arguments
nalimilan 6518c47
Also port map() to lift()
nalimilan a69bd5f
Fix broadcast() tests
nalimilan a1926c7
Small naming improvement
nalimilan 0eab4dc
Review comments
nalimilan 0a00622
Use generated function to avoid allocations on Julia 0.5
nalimilan 615d084
Simplify code and improve performance on Julia 0.5, use only generate…
nalimilan 75edfcb
Remove Julia 0.4 lines
nalimilan 2d8670d
Update eltype computation code to match Base
nalimilan 5f10b07
Fix deprecation warnings
nalimilan 83ea4cf
Fix 0.6 deprecations
nalimilan 3f17571
Require Compat 0.13.0
nalimilan 5a88dab
Fix computing eltype
nalimilan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Update eltype computation code to match Base
These have recently been simplified. map() in Base does not use the same path as broadcast(), but doing so here is much simpler and results should be equivalent for consistency anyway.
- Loading branch information
commit 2d8670d6e0b370489d385485c86c95f3dc1b49e0
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,16 +9,12 @@ else | |
const broadcast_indices = broadcast_shape | ||
end | ||
|
||
if !isdefined(Base.Broadcast, :ftype) # Julia < 0.6 | ||
if VERSION < v"0.6.0-dev" # Old approach needed for inference to work | ||
ftype(f, A) = typeof(f) | ||
ftype(f, A...) = typeof(a -> f(a...)) | ||
ftype(T::DataType, A) = Type{T} | ||
ftype(T::DataType, A...) = Type{T} | ||
else | ||
using Base.Broadcast: ftype | ||
end | ||
|
||
if !isdefined(Base.Broadcast, :ziptype) # Julia < 0.6 | ||
if isdefined(Base, :Iterators) | ||
using Base.Iterators: Zip2 | ||
else | ||
|
@@ -27,8 +23,19 @@ if !isdefined(Base.Broadcast, :ziptype) # Julia < 0.6 | |
ziptype(A) = Tuple{eltype(A)} | ||
ziptype(A, B) = Zip2{Tuple{eltype(A)}, Tuple{eltype(B)}} | ||
@inline ziptype(A, B, C, D...) = Zip{Tuple{eltype(A)}, ziptype(B, C, D...)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise here, the changes in JuliaLang/julia#19421 might be relevant. Best! |
||
|
||
nullable_broadcast_eltype(f, As...) = | ||
_default_eltype(Base.Generator{ziptype(As...), ftype(f, As...)}) | ||
else | ||
using Base.Broadcast: ziptype | ||
Base.@pure nullable_eltypestuple(a) = Tuple{eltype(eltype(a))} | ||
Base.@pure nullable_eltypestuple(T::Type) = Tuple{Type{eltype(T)}} | ||
Base.@pure nullable_eltypestuple(a, b...) = | ||
Tuple{nullable_eltypestuple(a).types..., nullable_eltypestuple(b...).types...} | ||
|
||
Base.@pure function nullable_broadcast_eltype(f, As...) | ||
T = Core.Inference.return_type(f, nullable_eltypestuple(As...)) | ||
T === Union{} ? Any : T | ||
end | ||
end | ||
|
||
invoke_broadcast!{F, N}(f::F, dest, As::Vararg{NullableArray, N}) = | ||
|
@@ -57,7 +64,7 @@ function Base.broadcast{F}(f::F, As::NullableArray...) | |
@inline f2(x1, x2, x3, x4, x5, x6, x7) = lift(f, (x1, x2, x3, x4, x5, x6, x7)) | ||
@inline f2(x...) = lift(f, x) | ||
|
||
T = _default_eltype(Base.Generator{ziptype(As...), ftype(f2, As...)}) | ||
T = nullable_broadcast_eltype(f, As...) | ||
dest = similar(NullableArray{eltype(T)}, broadcast_indices(As...)) | ||
invoke_broadcast!(f2, dest, As...) | ||
end | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in JuliaLang/julia#19421 might be relevant here. Best!