Skip to content

RFC reduce promotion: default to system default #20607

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

RFC reduce promotion: default to system default #20607

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2d17dd9
Merge pull request #2 from JuliaLang/master
felixrehren Feb 14, 2017
d58e889
promote type of `reduce` to system size
felixrehren Feb 14, 2017
0af7fe1
update tests
felixrehren Feb 14, 2017
5632f80
tests, II
felixrehren Feb 14, 2017
d2aeb2d
Float16 bumped in status, docs changed
felixrehren Feb 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions base/reduce.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,23 @@ const SmallUnsigned = Union{UInt8,UInt16,UInt32}
end

const CommonReduceResult = Union{UInt64,UInt128,Int64,Int128,Float32,Float64}
const WidenReduceResult = Union{SmallSigned, SmallUnsigned, Float16}
const SmallReduceResult = Union{SmallSigned, SmallUnsigned, Float16}
Copy link
Member

@stevengj stevengj Feb 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SmallReduceResult seems like a misnomer here. The result is Int, which is not small.

Float16 should still be widened to Float32, presumably, which is not a "system size". Or I guess the widening there could be eliminated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Float16 is a "real computational type" #5942 (comment) it shouldn't be widened, right?


promote_sys_size{T}(::Type{T}) = T
promote_sys_size{T<:SmallSigned}(::Type{T}) = Int
promote_sys_size{T<:SmallUnsigned}(::Type{T}) = UInt
# r_promote_type: promote T to the type of reduce(op, ::Array{T})
# (some "extra" methods are required here to avoid ambiguity warnings)
r_promote_type{T}(op, ::Type{T}) = T
r_promote_type{T<:WidenReduceResult}(op, ::Type{T}) = widen(T)
r_promote_type{T<:WidenReduceResult}(::typeof(+), ::Type{T}) = widen(T)
r_promote_type{T<:WidenReduceResult}(::typeof(*), ::Type{T}) = widen(T)
r_promote_type{T<:SmallReduceResult}(op, ::Type{T}) = promote_sys_size(T)
r_promote_type{T<:SmallReduceResult}(::typeof(+), ::Type{T}) = promote_sys_size(T)
r_promote_type{T<:SmallReduceResult}(::typeof(*), ::Type{T}) = promote_sys_size(T)
r_promote_type{T<:Number}(::typeof(+), ::Type{T}) = typeof(zero(T)+zero(T))
r_promote_type{T<:Number}(::typeof(*), ::Type{T}) = typeof(one(T)*one(T))
r_promote_type{T<:WidenReduceResult}(::typeof(scalarmax), ::Type{T}) = T
r_promote_type{T<:WidenReduceResult}(::typeof(scalarmin), ::Type{T}) = T
r_promote_type{T<:WidenReduceResult}(::typeof(max), ::Type{T}) = T
r_promote_type{T<:WidenReduceResult}(::typeof(min), ::Type{T}) = T
r_promote_type{T<:SmallReduceResult}(::typeof(scalarmax), ::Type{T}) = T
r_promote_type{T<:SmallReduceResult}(::typeof(scalarmin), ::Type{T}) = T
r_promote_type{T<:SmallReduceResult}(::typeof(max), ::Type{T}) = T
r_promote_type{T<:SmallReduceResult}(::typeof(min), ::Type{T}) = T

# r_promote: promote x to the type of reduce(op, [x])
r_promote{T}(op, x::T) = convert(r_promote_type(op, T), x)
Expand Down
18 changes: 9 additions & 9 deletions test/reduce.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# fold(l|r) & mapfold(l|r)
@test foldl(+, Int64[]) === Int64(0) # In reference to issues #7465/#20144 (PR #20160)
@test foldl(+, Int16[]) === Int32(0)
@test foldl(+, Int16[]) === Int(0)
@test foldl(-, 1:5) == -13
@test foldl(-, 10, 1:5) == -5

Expand All @@ -19,7 +19,7 @@
@test Base.mapfoldl((x)-> x ⊻ true, |, false, [true false true false false]) == true

@test foldr(+, Int64[]) === Int64(0) # In reference to issue #20144 (PR #20160)
@test foldr(+, Int16[]) === Int32(0)
@test foldr(+, Int16[]) === Int(0)
@test foldr(-, 1:5) == 3
@test foldr(-, 10, 1:5) == -7

Expand All @@ -28,7 +28,7 @@

# reduce & mapreduce
@test reduce(+, Int64[]) === Int64(0) # In reference to issue #20144 (PR #20160)
@test reduce(+, Int16[]) === Int32(0)
@test reduce(+, Int16[]) === Int(0)
@test reduce((x,y)->"($x+$y)", 9:11) == "((9+10)+11)"
@test reduce(max, [8 6 7 5 3 0 9]) == 9
@test reduce(+, 1000, 1:5) == (1000 + 1 + 2 + 3 + 4 + 5)
Expand All @@ -39,15 +39,15 @@

# sum

@test sum(Int8[]) === Int32(0)
@test sum(Int8[]) === Int(0)
@test sum(Int[]) === Int(0)
@test sum(Float64[]) === 0.0

@test sum(Int8(3)) === Int8(3)
@test sum(3) === 3
@test sum(3.0) === 3.0

@test sum([Int8(3)]) === Int32(3)
@test sum([Int8(3)]) === Int(3)
@test sum([3]) === 3
@test sum([3.0]) === 3.0

Expand Down Expand Up @@ -107,11 +107,11 @@ end
# prod

@test prod(Int[]) === 1
@test prod(Int8[]) === Int32(1)
@test prod(Int8[]) === Int(1)
@test prod(Float64[]) === 1.0

@test prod([3]) === 3
@test prod([Int8(3)]) === Int32(3)
@test prod([Int8(3)]) === Int(3)
@test prod([3.0]) === 3.0

@test prod(z) === 120
Expand All @@ -126,8 +126,8 @@ end
prod2(itr) = invoke(prod, Tuple{Any}, itr)
@test prod(Int[]) === prod2(Int[]) === 1
@test prod(Int[7]) === prod2(Int[7]) === 7
@test typeof(prod(Int8[])) == typeof(prod(Int8[1])) == typeof(prod(Int8[1, 7])) == Int32
@test typeof(prod2(Int8[])) == typeof(prod2(Int8[1])) == typeof(prod2(Int8[1 7])) == Int32
@test typeof(prod(Int8[])) == typeof(prod(Int8[1])) == typeof(prod(Int8[1, 7])) == Int
@test typeof(prod2(Int8[])) == typeof(prod2(Int8[1])) == typeof(prod2(Int8[1 7])) == Int

# maximum & minimum & extrema

Expand Down