Merge pull request #5 from SymbolicML/protect-overflow

Error on overflow in dimensions variable

Author: MilesCranmer

Project.toml

@@ -6,6 +6,7 @@ version = "0.1.0"
 [deps]
 Ratios = "c84ed2f1-dad5-54f0-aa8e-dbefe2724439"
 Requires = "ae029012-a4dd-5104-9daa-d747884805df"
+SaferIntegers = "88634af6-177f-5301-88b8-7819386cfa38"
 
 [weakdeps]
 Unitful = "1986cc42-f94f-5a68-af5c-568840ba703d"
@@ -16,6 +17,7 @@ DynamicQuantitiesUnitfulExt = "Unitful"
 [compat]
 Ratios = "0.4"
 Requires = "1"
+SaferIntegers = "3"
 Unitful = "1"
 julia = "1.6"
 

src/math.jl

@@ -20,13 +20,13 @@ Base.:+(l::Quantity, r::Quantity) = Quantity(l.value + r.value, l.dimensions, l.
 Base.:-(l::Quantity, r::Quantity) = Quantity(l.value - r.value, l.dimensions, l.valid && r.valid && l.dimensions == r.dimensions)
 
 Base.:^(l::Quantity, r::Quantity) =
-    let rr = tryrationalize(Int, r.value)
+    let rr = tryrationalize(R, r.value)
         Quantity(l.value^rr, l.dimensions^rr, l.valid && r.valid && iszero(r.dimensions))
     end
 Base.:^(l::Dimensions, r::R) = @map_dimensions(Base.Fix1(*, r), l)
-Base.:^(l::Dimensions, r::Number) = l^tryrationalize(Int, r)
+Base.:^(l::Dimensions, r::Number) = l^tryrationalize(R, r)
 Base.:^(l::Quantity, r::Number) =
-    let rr = tryrationalize(Int, r)
+    let rr = tryrationalize(R, r)
         Quantity(l.value^rr, l.dimensions^rr, l.valid)
     end
 

src/types.jl

@@ -1,6 +1,9 @@
 import Ratios: SimpleRatio
+import SaferIntegers: SafeInt
 
-const R = SimpleRatio{Int}
+const INT_TYPE = SafeInt
+const R = SimpleRatio{INT_TYPE}
+const ZERO = R(0)
 const DIMENSION_NAMES = (:length, :mass, :time, :current, :temperature, :luminosity, :amount)
 const DIMENSION_SYNONYMS = (:𝐋, :𝐌, :𝐓, :𝐈, :𝚯, :𝐉, :𝐍)
 const SYNONYM_MAPPING = NamedTuple(DIMENSION_NAMES .=> DIMENSION_SYNONYMS)
@@ -34,13 +37,13 @@ struct Dimensions
     Dimensions(length::R, mass::R, time::R, current::R, temperature::R, luminosity::R, amount::R) =
         new(length, mass, time, current, temperature, luminosity, amount)
     Dimensions(; kws...) = Dimensions(
-        tryrationalize(Int, get(kws, :length, 0 // 1)),
-        tryrationalize(Int, get(kws, :mass, 0 // 1)),
-        tryrationalize(Int, get(kws, :time, 0 // 1)),
-        tryrationalize(Int, get(kws, :current, 0 // 1)),
-        tryrationalize(Int, get(kws, :temperature, 0 // 1)),
-        tryrationalize(Int, get(kws, :luminosity, 0 // 1)),
-        tryrationalize(Int, get(kws, :amount, 0 // 1)),
+        tryrationalize(R, get(kws, :length, ZERO)),
+        tryrationalize(R, get(kws, :mass, ZERO)),
+        tryrationalize(R, get(kws, :time, ZERO)),
+        tryrationalize(R, get(kws, :current, ZERO)),
+        tryrationalize(R, get(kws, :temperature, ZERO)),
+        tryrationalize(R, get(kws, :luminosity, ZERO)),
+        tryrationalize(R, get(kws, :amount, ZERO)),
     )
 end
 

src/utils.jl

@@ -79,10 +79,14 @@ to_superscript(s::AbstractString) = join(
     end
 )
 
-tryrationalize(::Type{<:Integer}, x::Rational) = R(x)
-tryrationalize(::Type{<:Integer}, x::Integer) = R(x)
-tryrationalize(::Type{<:Integer}, x) = simple_ratio_rationalize(x)
-simple_ratio_rationalize(x) = isinteger(x) ? R(round(Int, x)) : R(rationalize(Int, x))
+tryrationalize(::Type{RI}, x::RI) where {RI} = x
+tryrationalize(::Type{RI}, x::Rational) where {RI} = RI(x)
+tryrationalize(::Type{RI}, x::Integer) where {RI} = RI(x)
+tryrationalize(::Type{RI}, x) where {RI} = simple_ratio_rationalize(RI, x)
+simple_ratio_rationalize(::Type{RI}, x) where {RI} =
+    let int_type = RI.parameters[1]
+        isinteger(x) ? RI(round(int_type, x)) : RI(rationalize(int_type, x))
+    end
 
 """
     ustrip(q::Quantity)

test/unittests.jl

@@ -1,4 +1,5 @@
 using DynamicQuantities
+using DynamicQuantities: INT_TYPE, R
 using Test
 
 @testset "Basic utilities" begin
@@ -149,4 +150,18 @@ end
     d = Dimensions(length=-0.2, luminosity=2)
     q = Quantity(0.5, inv(d))
     @test q == Quantity(0.5, length=0.2, luminosity=-2)
+end
+
+@testset "Trigger overflow" begin
+    f(x, N) =
+        let
+            for _ = 1:N
+                x = x + R(2 // 3)
+                x = x - R(2 // 3)
+            end
+            x
+        end
+
+    @test f(R(5 // 7), 15) == R(5 // 7)
+    @test_throws OverflowError f(R(5 // 7), 20)
 end