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

Don't throw InexactError for comparisons between FDs of different types and/or Integers #88

Merged
merged 13 commits into from
Dec 19, 2023
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "FixedPointDecimals"
uuid = "fb4d412d-6eee-574d-9565-ede6634db7b0"
authors = ["Fengyang Wang <fengyang.wang.0@gmail.com>", "Curtis Vogt <curtis.vogt@gmail.com>"]
version = "0.5.0"
version = "0.5.1"

[deps]
Parsers = "69de0a69-1ddd-5017-9359-2bf0b02dc9f0"
Expand Down
99 changes: 96 additions & 3 deletions src/FixedPointDecimals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -440,12 +440,14 @@ end

function Base.checked_neg(x::T) where {T<:FD}
r = -x
(x<0) & (r<0) && Base.Checked.throw_overflowerr_negation(x)
# Simplify the compiler's job, no need to call the FD,Int comparison
(x.i<0) & (r.i<0) && Base.Checked.throw_overflowerr_negation(x)
return r
end
function Base.checked_abs(x::FD)
r = ifelse(x<0, -x, x)
r<0 || return r
# Simplify the compiler's job, no need to call the FD,Int comparison
r = ifelse(x.i<0, -x, x)
r.i<0 || return r
_throw_overflow_abs(x)
end
if VERSION >= v"1.8.0-"
Expand Down Expand Up @@ -537,6 +539,97 @@ Base.:(==)(x::T, y::T) where {T <: FD} = x.i == y.i
Base.:(<)(x::T, y::T) where {T <: FD} = x.i < y.i
Base.:(<=)(x::T, y::T) where {T <: FD} = x.i <= y.i

# In the case where the FD types are different, or where one is an Integer, add
# custom overloads, rather than relying on the default promotions, to avoid throwing
# InexactError from the promotions. This lets us do e.g. `FD{Int8,2}(0) < 2`, without the
# promotion to FD{Int8,2}(2) throwing an InexactError.
for comp_op in (:(==), :(<), :(<=))
NHDaly marked this conversation as resolved.
Show resolved Hide resolved
@eval function Base.$comp_op(x::FD{T1,f1}, y::FD{T2,f2}) where {T1<:Integer, f1, T2<:Integer, f2}
if f1 == f2
# If the precisions are the same, even if the types are different, we can compare
# the integer values directly. e.g. Int8(2) == Int16(2) is true.
return $comp_op(x.i, y.i)
else
# Promote the integers to the larger type, and then scale them to the same
# precision. If the scaling operation ends up overflowing, we know that they aren't
# equal, because we know that the less precise value wasn't even representable in
# the more precise type, so they cannot be equal.
newFD = promote_type(FD{T1,f1}, FD{T2,f2})
xi, yi = promote(x.i, y.i)
if f1 > f2
C = coefficient(newFD) ÷ coefficient(FD{T2,f2})
yi, overflowed = Base.mul_with_overflow(yi, C)
if $(comp_op == :(==))
overflowed && return false
else
# y overflowed, so it's bigger than x, since it doesn't fit in x's type
overflowed && return true
end
else
C = coefficient(newFD) ÷ coefficient(FD{T1,f1})
xi, overflowed = Base.mul_with_overflow(xi, C)
# Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's
# bigger than y, so this is false.
overflowed && return false
end
return $comp_op(xi, yi)
end
end
@eval function Base.$comp_op(x::Integer, y::FD{T,f}) where {T<:Integer, f}
if f == 0
# If the precisions are the same, even if the types are different, we can
# compare the integer values directly. e.g. Int8(2) == Int16(2) is true.
return $comp_op(x, y.i)
else
if !(x isa T)
if x > typemax(T)
# If x is too big to fit in T, then we know already that it's bigger
# than y, so not equal and not less than.
return false
elseif x < typemin(T)
# Similarly, if too small, it's definitely less than y (and not equal).
return $(comp_op == :(==)) ? false : true
end
end
# Now it's safe to truncate x down to y's type.
xi = x % T
xi, overflowed = Base.mul_with_overflow(xi, coefficient(FD{T,f}))
# Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's
# bigger than y, so this is false.
overflowed && return false
return $comp_op(xi, y.i)
end
end
@eval function Base.$comp_op(x::FD{T,f}, y::Integer) where {T<:Integer, f}
if f == 0
# If the precisions are the same, even if the types are different, we can
# compare the integer values directly. e.g. Int8(2) == Int16(2) is true.
return $comp_op(x.i, y)
else
if !(y isa T)
if y > typemax(T)
# If y is too big to fit in T, then we know already that x is smaller
# than y. So not equal, but definitely x < y.
return $(comp_op == :(==)) ? false : true
elseif y < typemin(T)
# Similarly, if y is too small, definitely x > y (and not equal).
return false
end
end
# Now it's safe to truncate x down to y's type.
yi = y % T
yi, overflowed = Base.mul_with_overflow(yi, coefficient(FD{T,f}))
if $(comp_op == :(==))
overflowed && return false
else
# y overflowed, so it's bigger than x, since it doesn't fit in x's type
overflowed && return true
end
return $comp_op(x.i, yi)
end
end
end

# predicates and traits
Base.isinteger(x::FD{T, f}) where {T, f} = rem(x.i, coefficient(FD{T, f})) == 0
Base.typemin(::Type{FD{T, f}}) where {T, f} = reinterpret(FD{T, f}, typemin(T))
Expand Down
96 changes: 96 additions & 0 deletions test/FixedDecimal.jl
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,102 @@ end
end
end

@testset "equality between types" begin
@test FD{Int8, 0}(1) == FD{Int8, 2}(1)
@test FD{Int8, 0}(0) != FD{Int8, 2}(1)
# Note: this doesn't throw inexact error
@test FD{Int8, 0}(2) != FD{Int8, 2}(1) # FD{Int8,2}(2) doesn't fit

@test FD{Int8, 0}(1) == FD{Int16, 1}(1)
@test FD{Int8, 0}(2) != FD{Int16, 1}(1)
# Note: this doesn't throw inexact error
@test FD{Int8, 0}(4) != FD{Int16, 4}(1) # FD{Int16,4}(4) doesn't fit

# Integer == FD
@test 1 == FD{Int8, 2}(1)
@test 2 != FD{Int8, 2}(1)
@test FD{Int8, 2}(1) == 1
@test FD{Int8, 2}(1) != 2
@test 1 == FD{Int8, 0}(1) != 2

@test typemax(Int16) !== FD{Int8, 0}(1)
@test typemax(Int16) !== FD{Int8, 2}(1)
@test typemin(Int16) !== FD{Int8, 0}(1)
@test typemin(Int16) !== FD{Int8, 2}(1)
@test FD{Int8, 0}(1) != typemax(Int16)
@test FD{Int8, 2}(1) != typemax(Int16)
@test FD{Int8, 0}(1) != typemin(Int16)
@test FD{Int8, 2}(1) != typemin(Int16)

@test typemax(Int16) !== FD{Int8, 0}(-1)
@test typemax(Int16) !== FD{Int8, 2}(-1)
@test typemin(Int16) !== FD{Int8, 0}(-1)
@test typemin(Int16) !== FD{Int8, 2}(-1)
@test FD{Int8, 0}(-1) != typemax(Int16)
@test FD{Int8, 2}(-1) != typemax(Int16)
@test FD{Int8, 0}(-1) != typemin(Int16)
@test FD{Int8, 2}(-1) != typemin(Int16)

@test typemax(Int16) !== FD{Int8, 0}(0)
@test typemax(Int16) !== FD{Int8, 2}(0)
@test typemin(Int16) !== FD{Int8, 0}(0)
@test typemin(Int16) !== FD{Int8, 2}(0)
@test FD{Int8, 0}(0) != typemax(Int16)
@test FD{Int8, 2}(0) != typemax(Int16)
@test FD{Int8, 0}(0) != typemin(Int16)
@test FD{Int8, 2}(0) != typemin(Int16)
end
@testset "inequality between types" begin
@test FD{Int8, 0}(1) <= FD{Int8, 2}(1)
@test FD{Int8, 0}(0) < FD{Int8, 2}(1)
# Note: this doesn't throw inexact error
@test FD{Int8, 0}(2) >= FD{Int8, 2}(1) # FD{Int8,2}(2) doesn't fit
@test FD{Int8, 2}(1) < FD{Int8, 0}(2) # FD{Int8,2}(2) doesn't fit

@test FD{Int8, 0}(1) <= FD{Int16, 1}(1)
@test FD{Int8, 0}(2) > FD{Int16, 1}(1)
# Note: this doesn't throw inexact error
@test FD{Int8, 0}(4) > FD{Int16, 4}(1) # FD{Int16,4}(4) doesn't fit
@test FD{Int8, 0}(4) >= FD{Int16, 4}(1) # FD{Int16,4}(4) doesn't fit
@test FD{Int16, 4}(1) < FD{Int8, 0}(4) # FD{Int16,4}(4) doesn't fit

# Integer == FD
@test 1 <= FD{Int8, 2}(1) <= 1
@test 1 >= FD{Int8, 2}(1) >= 1
@test 2 > FD{Int8, 2}(1)
@test FD{Int8, 2}(1) < 2
@test 2 >= FD{Int8, 2}(1)
@test FD{Int8, 2}(1) <= 2
@test 1 <= FD{Int8, 0}(1) < 2

@test typemax(Int16) > FD{Int8, 0}(1) > typemin(Int16)
@test typemax(Int16) > FD{Int8, 2}(1) > typemin(Int16)
@test typemin(Int16) < FD{Int8, 0}(1) < typemax(Int16)
@test typemin(Int16) < FD{Int8, 2}(1) < typemax(Int16)
@test !(typemax(Int16) < FD{Int8, 0}(1) < typemin(Int16))
@test !(typemax(Int16) < FD{Int8, 2}(1) < typemin(Int16))
@test !(typemin(Int16) > FD{Int8, 0}(1) > typemax(Int16))
@test !(typemin(Int16) > FD{Int8, 2}(1) > typemax(Int16))

@test typemax(Int16) > FD{Int8, 0}(-1) > typemin(Int16)
@test typemax(Int16) > FD{Int8, 2}(-1) > typemin(Int16)
@test typemin(Int16) < FD{Int8, 0}(-1) < typemax(Int16)
@test typemin(Int16) < FD{Int8, 2}(-1) < typemax(Int16)
@test !(typemax(Int16) < FD{Int8, 0}(-1) < typemin(Int16))
@test !(typemax(Int16) < FD{Int8, 2}(-1) < typemin(Int16))
@test !(typemin(Int16) > FD{Int8, 0}(-1) > typemax(Int16))
@test !(typemin(Int16) > FD{Int8, 2}(-1) > typemax(Int16))

@test typemax(Int16) >= FD{Int8, 0}(0) >= typemin(Int16)
@test typemax(Int16) >= FD{Int8, 2}(0) >= typemin(Int16)
@test typemin(Int16) <= FD{Int8, 0}(0) <= typemax(Int16)
@test typemin(Int16) <= FD{Int8, 2}(0) <= typemax(Int16)
@test !(typemax(Int16) <= FD{Int8, 0}(-1) <= typemin(Int16))
@test !(typemax(Int16) <= FD{Int8, 2}(-1) <= typemin(Int16))
@test !(typemin(Int16) >= FD{Int8, 0}(-1) >= typemax(Int16))
@test !(typemin(Int16) >= FD{Int8, 2}(-1) >= typemax(Int16))
end

@testset "128-bit conversion correctness" begin
# Force the bits for these tests
F64D2 = FixedDecimal{Int64, 2}
Expand Down
Loading