Skip to content

Commit

Permalink
Don't throw InexactError for comparisons between FDs of different typ…
Browse files Browse the repository at this point in the history 
…es and/or Integers (#88)

* Fix InexactErrors for equality comparisons

* Handle inequalities as well! :)

Code generation to the rescue.

* Add comment about `x.i` comparisons in negative / abs checks

* Patch version bump: v0.5.1
  • Loading branch information
@NHDaly
NHDaly authored Dec 19, 2023
1 parent e769be4 commit 507185c
Show file tree
Hide file tree
Showing 3 changed files with 193 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Project.toml
View file
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 <[email protected]>", "Curtis Vogt <[email protected]>"]
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
View file
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 (:(==), :(<), :(<=))
@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
View file
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

2 comments on commit 507185c

@NHDaly
Copy link
Member Author

@NHDaly NHDaly commented on 507185c Dec 19, 2023

Choose a reason for hiding this comment

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

@JuliaRegistrator register()

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request created: JuliaRegistries/General/97444

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.5.1 -m "<description of version>" 507185c4e03f825a0776550260b8c352a5374443
git push origin v0.5.1

Please sign in to comment.