Skip to content

Commit 507185c

Browse files
NHDalyNHDaly
authored
Don't throw InexactError for comparisons between FDs of different types 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
1 parent e769be4 commit 507185c

File tree

3 files changed

+193
-4
lines changed

3 files changed

+193
-4
lines changed

‎Project.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name = "FixedPointDecimals"
22
uuid = "fb4d412d-6eee-574d-9565-ede6634db7b0"
33
authors = ["Fengyang Wang <fengyang.wang.0@gmail.com>", "Curtis Vogt <curtis.vogt@gmail.com>"]
4-
version = "0.5.0"
4+
version = "0.5.1"
55

66
[deps]
77
Parsers = "69de0a69-1ddd-5017-9359-2bf0b02dc9f0"

‎src/FixedPointDecimals.jl

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -440,12 +440,14 @@ end
440440

441441
function Base.checked_neg(x::T) where {T<:FD}
442442
r = -x
443-
(x<0) & (r<0) && Base.Checked.throw_overflowerr_negation(x)
443+
# Simplify the compiler's job, no need to call the FD,Int comparison
444+
(x.i<0) & (r.i<0) && Base.Checked.throw_overflowerr_negation(x)
444445
return r
445446
end
446447
function Base.checked_abs(x::FD)
447-
r = ifelse(x<0, -x, x)
448-
r<0 || return r
448+
# Simplify the compiler's job, no need to call the FD,Int comparison
449+
r = ifelse(x.i<0, -x, x)
450+
r.i<0 || return r
449451
_throw_overflow_abs(x)
450452
end
451453
if VERSION >= v"1.8.0-"
@@ -537,6 +539,97 @@ Base.:(==)(x::T, y::T) where {T <: FD} = x.i == y.i
537539
Base.:(<)(x::T, y::T) where {T <: FD} = x.i < y.i
538540
Base.:(<=)(x::T, y::T) where {T <: FD} = x.i <= y.i
539541

542+
# In the case where the FD types are different, or where one is an Integer, add
543+
# custom overloads, rather than relying on the default promotions, to avoid throwing
544+
# InexactError from the promotions. This lets us do e.g. `FD{Int8,2}(0) < 2`, without the
545+
# promotion to FD{Int8,2}(2) throwing an InexactError.
546+
for comp_op in (:(==), :(<), :(<=))
547+
@eval function Base.$comp_op(x::FD{T1,f1}, y::FD{T2,f2}) where {T1<:Integer, f1, T2<:Integer, f2}
548+
if f1 == f2
549+
# If the precisions are the same, even if the types are different, we can compare
550+
# the integer values directly. e.g. Int8(2) == Int16(2) is true.
551+
return $comp_op(x.i, y.i)
552+
else
553+
# Promote the integers to the larger type, and then scale them to the same
554+
# precision. If the scaling operation ends up overflowing, we know that they aren't
555+
# equal, because we know that the less precise value wasn't even representable in
556+
# the more precise type, so they cannot be equal.
557+
newFD = promote_type(FD{T1,f1}, FD{T2,f2})
558+
xi, yi = promote(x.i, y.i)
559+
if f1 > f2
560+
C = coefficient(newFD) ÷ coefficient(FD{T2,f2})
561+
yi, overflowed = Base.mul_with_overflow(yi, C)
562+
if $(comp_op == :(==))
563+
overflowed && return false
564+
else
565+
# y overflowed, so it's bigger than x, since it doesn't fit in x's type
566+
overflowed && return true
567+
end
568+
else
569+
C = coefficient(newFD) ÷ coefficient(FD{T1,f1})
570+
xi, overflowed = Base.mul_with_overflow(xi, C)
571+
# Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's
572+
# bigger than y, so this is false.
573+
overflowed && return false
574+
end
575+
return $comp_op(xi, yi)
576+
end
577+
end
578+
@eval function Base.$comp_op(x::Integer, y::FD{T,f}) where {T<:Integer, f}
579+
if f == 0
580+
# If the precisions are the same, even if the types are different, we can
581+
# compare the integer values directly. e.g. Int8(2) == Int16(2) is true.
582+
return $comp_op(x, y.i)
583+
else
584+
if !(x isa T)
585+
if x > typemax(T)
586+
# If x is too big to fit in T, then we know already that it's bigger
587+
# than y, so not equal and not less than.
588+
return false
589+
elseif x < typemin(T)
590+
# Similarly, if too small, it's definitely less than y (and not equal).
591+
return $(comp_op == :(==)) ? false : true
592+
end
593+
end
594+
# Now it's safe to truncate x down to y's type.
595+
xi = x % T
596+
xi, overflowed = Base.mul_with_overflow(xi, coefficient(FD{T,f}))
597+
# Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's
598+
# bigger than y, so this is false.
599+
overflowed && return false
600+
return $comp_op(xi, y.i)
601+
end
602+
end
603+
@eval function Base.$comp_op(x::FD{T,f}, y::Integer) where {T<:Integer, f}
604+
if f == 0
605+
# If the precisions are the same, even if the types are different, we can
606+
# compare the integer values directly. e.g. Int8(2) == Int16(2) is true.
607+
return $comp_op(x.i, y)
608+
else
609+
if !(y isa T)
610+
if y > typemax(T)
611+
# If y is too big to fit in T, then we know already that x is smaller
612+
# than y. So not equal, but definitely x < y.
613+
return $(comp_op == :(==)) ? false : true
614+
elseif y < typemin(T)
615+
# Similarly, if y is too small, definitely x > y (and not equal).
616+
return false
617+
end
618+
end
619+
# Now it's safe to truncate x down to y's type.
620+
yi = y % T
621+
yi, overflowed = Base.mul_with_overflow(yi, coefficient(FD{T,f}))
622+
if $(comp_op == :(==))
623+
overflowed && return false
624+
else
625+
# y overflowed, so it's bigger than x, since it doesn't fit in x's type
626+
overflowed && return true
627+
end
628+
return $comp_op(x.i, yi)
629+
end
630+
end
631+
end
632+
540633
# predicates and traits
541634
Base.isinteger(x::FD{T, f}) where {T, f} = rem(x.i, coefficient(FD{T, f})) == 0
542635
Base.typemin(::Type{FD{T, f}}) where {T, f} = reinterpret(FD{T, f}, typemin(T))

‎test/FixedDecimal.jl

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,102 @@ end
253253
end
254254
end
255255

256+
@testset "equality between types" begin
257+
@test FD{Int8, 0}(1) == FD{Int8, 2}(1)
258+
@test FD{Int8, 0}(0) != FD{Int8, 2}(1)
259+
# Note: this doesn't throw inexact error
260+
@test FD{Int8, 0}(2) != FD{Int8, 2}(1) # FD{Int8,2}(2) doesn't fit
261+
262+
@test FD{Int8, 0}(1) == FD{Int16, 1}(1)
263+
@test FD{Int8, 0}(2) != FD{Int16, 1}(1)
264+
# Note: this doesn't throw inexact error
265+
@test FD{Int8, 0}(4) != FD{Int16, 4}(1) # FD{Int16,4}(4) doesn't fit
266+
267+
# Integer == FD
268+
@test 1 == FD{Int8, 2}(1)
269+
@test 2 != FD{Int8, 2}(1)
270+
@test FD{Int8, 2}(1) == 1
271+
@test FD{Int8, 2}(1) != 2
272+
@test 1 == FD{Int8, 0}(1) != 2
273+
274+
@test typemax(Int16) !== FD{Int8, 0}(1)
275+
@test typemax(Int16) !== FD{Int8, 2}(1)
276+
@test typemin(Int16) !== FD{Int8, 0}(1)
277+
@test typemin(Int16) !== FD{Int8, 2}(1)
278+
@test FD{Int8, 0}(1) != typemax(Int16)
279+
@test FD{Int8, 2}(1) != typemax(Int16)
280+
@test FD{Int8, 0}(1) != typemin(Int16)
281+
@test FD{Int8, 2}(1) != typemin(Int16)
282+
283+
@test typemax(Int16) !== FD{Int8, 0}(-1)
284+
@test typemax(Int16) !== FD{Int8, 2}(-1)
285+
@test typemin(Int16) !== FD{Int8, 0}(-1)
286+
@test typemin(Int16) !== FD{Int8, 2}(-1)
287+
@test FD{Int8, 0}(-1) != typemax(Int16)
288+
@test FD{Int8, 2}(-1) != typemax(Int16)
289+
@test FD{Int8, 0}(-1) != typemin(Int16)
290+
@test FD{Int8, 2}(-1) != typemin(Int16)
291+
292+
@test typemax(Int16) !== FD{Int8, 0}(0)
293+
@test typemax(Int16) !== FD{Int8, 2}(0)
294+
@test typemin(Int16) !== FD{Int8, 0}(0)
295+
@test typemin(Int16) !== FD{Int8, 2}(0)
296+
@test FD{Int8, 0}(0) != typemax(Int16)
297+
@test FD{Int8, 2}(0) != typemax(Int16)
298+
@test FD{Int8, 0}(0) != typemin(Int16)
299+
@test FD{Int8, 2}(0) != typemin(Int16)
300+
end
301+
@testset "inequality between types" begin
302+
@test FD{Int8, 0}(1) <= FD{Int8, 2}(1)
303+
@test FD{Int8, 0}(0) < FD{Int8, 2}(1)
304+
# Note: this doesn't throw inexact error
305+
@test FD{Int8, 0}(2) >= FD{Int8, 2}(1) # FD{Int8,2}(2) doesn't fit
306+
@test FD{Int8, 2}(1) < FD{Int8, 0}(2) # FD{Int8,2}(2) doesn't fit
307+
308+
@test FD{Int8, 0}(1) <= FD{Int16, 1}(1)
309+
@test FD{Int8, 0}(2) > FD{Int16, 1}(1)
310+
# Note: this doesn't throw inexact error
311+
@test FD{Int8, 0}(4) > FD{Int16, 4}(1) # FD{Int16,4}(4) doesn't fit
312+
@test FD{Int8, 0}(4) >= FD{Int16, 4}(1) # FD{Int16,4}(4) doesn't fit
313+
@test FD{Int16, 4}(1) < FD{Int8, 0}(4) # FD{Int16,4}(4) doesn't fit
314+
315+
# Integer == FD
316+
@test 1 <= FD{Int8, 2}(1) <= 1
317+
@test 1 >= FD{Int8, 2}(1) >= 1
318+
@test 2 > FD{Int8, 2}(1)
319+
@test FD{Int8, 2}(1) < 2
320+
@test 2 >= FD{Int8, 2}(1)
321+
@test FD{Int8, 2}(1) <= 2
322+
@test 1 <= FD{Int8, 0}(1) < 2
323+
324+
@test typemax(Int16) > FD{Int8, 0}(1) > typemin(Int16)
325+
@test typemax(Int16) > FD{Int8, 2}(1) > typemin(Int16)
326+
@test typemin(Int16) < FD{Int8, 0}(1) < typemax(Int16)
327+
@test typemin(Int16) < FD{Int8, 2}(1) < typemax(Int16)
328+
@test !(typemax(Int16) < FD{Int8, 0}(1) < typemin(Int16))
329+
@test !(typemax(Int16) < FD{Int8, 2}(1) < typemin(Int16))
330+
@test !(typemin(Int16) > FD{Int8, 0}(1) > typemax(Int16))
331+
@test !(typemin(Int16) > FD{Int8, 2}(1) > typemax(Int16))
332+
333+
@test typemax(Int16) > FD{Int8, 0}(-1) > typemin(Int16)
334+
@test typemax(Int16) > FD{Int8, 2}(-1) > typemin(Int16)
335+
@test typemin(Int16) < FD{Int8, 0}(-1) < typemax(Int16)
336+
@test typemin(Int16) < FD{Int8, 2}(-1) < typemax(Int16)
337+
@test !(typemax(Int16) < FD{Int8, 0}(-1) < typemin(Int16))
338+
@test !(typemax(Int16) < FD{Int8, 2}(-1) < typemin(Int16))
339+
@test !(typemin(Int16) > FD{Int8, 0}(-1) > typemax(Int16))
340+
@test !(typemin(Int16) > FD{Int8, 2}(-1) > typemax(Int16))
341+
342+
@test typemax(Int16) >= FD{Int8, 0}(0) >= typemin(Int16)
343+
@test typemax(Int16) >= FD{Int8, 2}(0) >= typemin(Int16)
344+
@test typemin(Int16) <= FD{Int8, 0}(0) <= typemax(Int16)
345+
@test typemin(Int16) <= FD{Int8, 2}(0) <= typemax(Int16)
346+
@test !(typemax(Int16) <= FD{Int8, 0}(-1) <= typemin(Int16))
347+
@test !(typemax(Int16) <= FD{Int8, 2}(-1) <= typemin(Int16))
348+
@test !(typemin(Int16) >= FD{Int8, 0}(-1) >= typemax(Int16))
349+
@test !(typemin(Int16) >= FD{Int8, 2}(-1) >= typemax(Int16))
350+
end
351+
256352
@testset "128-bit conversion correctness" begin
257353
# Force the bits for these tests
258354
F64D2 = FixedDecimal{Int64, 2}

0 commit comments

Comments
 (0)