Skip to content

Commit

Permalink
Fixed rounding such that it doesn't overflow in edge cases. Also adde…
Browse files Browse the repository at this point in the history
…d tests. Fixes JuliaLang#15215.
  • Loading branch information
pkofod committed Feb 24, 2016
1 parent f5338c8 commit 1c7da9a
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
6 changes: 3 additions & 3 deletions base/rational.jl
Original file line number Diff line number Diff line change
Expand Up @@ -290,18 +290,18 @@ ceil{ T}(::Type{T}, x::Rational) = convert(T,cld(x.num,x.den))

function round{T}(::Type{T}, x::Rational, ::RoundingMode{:Nearest})
q,r = divrem(x.num,x.den)
s = abs(r) < (x.den+one(x.den)+iseven(q))>>1 ? q : q+copysign(one(q),x.num)
s = abs(r) < abs((x.den-copysign(typeof(x.den)(4), x.num)+one(x.den)+iseven(q))>>1 + copysign(typeof(x.den)(2), x.num)) ? q : q+copysign(one(q),x.num)

This comment has been minimized.

Copy link
@tkelman

tkelman Feb 25, 2016

these lines are getting a bit long, maybe split up the ternary into an if ?

convert(T,s)
end
round{T}(::Type{T}, x::Rational) = round(T,x,RoundNearest)
function round{T}(::Type{T}, x::Rational, ::RoundingMode{:NearestTiesAway})
q,r = divrem(x.num,x.den)
s = abs(r) < (x.den+one(x.den))>>1 ? q : q+copysign(one(q),x.num)
s = abs(r) < abs((x.den-copysign(typeof(x.den)(4), x.num)+one(x.den))>>1 + copysign(typeof(x.den)(2), x.num)) ? q : q+copysign(one(q),x.num)
convert(T,s)
end
function round{T}(::Type{T}, x::Rational, ::RoundingMode{:NearestTiesUp})
q,r = divrem(x.num,x.den)
s = abs(r) < (x.den+one(x.den)+(x.num<0))>>1 ? q : q+copysign(one(q),x.num)
s = abs(r) < abs((x.den-copysign(typeof(x.den)(4), x.num)+one(x.den)+(x.num<0))>>1 + copysign(typeof(x.den)(2), x.num)) ? q : q+copysign(one(q),x.num)
convert(T,s)
end

Expand Down
22 changes: 16 additions & 6 deletions test/numbers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2782,12 +2782,12 @@ for Tf = (Float16, Float32, Float64), Ti = (Int16, Int32, Int64)
@test round(Tf, Rational(1,2), RoundNearestTiesUp) == 1.0
@test round(Tf, Rational(1,2), RoundNearestTiesAway) == 1.0

# @test round( almost_half) == 0//1
# @test round(-almost_half) == 0//1
# @test round(Tf, almost_half, RoundNearestTiesUp) == 0.0
# @test round(Tf, -almost_half, RoundNearestTiesUp) == 0.0
# @test round(Tf, almost_half, RoundNearestTiesAway) == 0.0
# @test round(Tf, -almost_half, RoundNearestTiesAway) == 0.0
@test round( almost_half) == 0//1
@test round(-almost_half) == 0//1
@test round(Tf, almost_half, RoundNearestTiesUp) == 0.0
@test round(Tf, -almost_half, RoundNearestTiesUp) == 0.0
@test round(Tf, almost_half, RoundNearestTiesAway) == 0.0
@test round(Tf, -almost_half, RoundNearestTiesAway) == 0.0

@test round( exactly_half) == 0//1 # rounds to closest _even_ integer
@test round(-exactly_half) == 0//1 # rounds to closest _even_ integer
Expand All @@ -2803,6 +2803,11 @@ for Tf = (Float16, Float32, Float64), Ti = (Int16, Int32, Int64)
@test round(Tf, over_half, RoundNearestTiesAway) == 1.0
@test round(Tf, -over_half, RoundNearestTiesUp) == -1.0
@test round(Tf, -over_half, RoundNearestTiesAway) == -1.0

@test round(Tf, 11//2, RoundNearestTiesUp) == 6.0
@test round(Tf, -11//2, RoundNearestTiesUp) == -5.0
@test round(Tf, 11//2, RoundNearestTiesAway) == 6.0
@test round(Tf, -11//2, RoundNearestTiesAway) == -6.0
end

let
Expand All @@ -2823,3 +2828,8 @@ let
@test read(io2, typeof(rational2)) == rational2
end
end

@test round(11//2) == 6 # rounds to closest _even_ integer
@test round(-11//2) == -6 # rounds to closest _even_ integer
@test round(11//3) == 4 # rounds to closest _even_ integer
@test round(-11//3) == -4 # rounds to closest _even_ integer

0 comments on commit 1c7da9a

Please sign in to comment.