Compose SimpleRatio and SaferIntegers

[mkitti]

This pull request composes Ratios with SaferIntegers in order to detect overflow errors.

SafeSigned and SafeUnsigned in SaferIntegers are not subtypes of Signed and Unsigned, respectively. Therefore, the additive inverse method is not defined in Ratios.jl for these types resulting in a MethodError.

SafeSigned and SafeUnsignedare subtypes of SafeInteger and Integer, so the other methods are defined.

julia> using Ratios, SaferIntegers

julia> -SimpleRatio{SafeInt}(1,5)
ERROR: MethodError: no method matching -(::SimpleRatio{SafeInt64})
...
julia> -SimpleRatio{SafeUInt}(1,5)
ERROR: MethodError: no method matching -(::SimpleRatio{SafeUInt64})
...

This pull request adds methods to take the additive inverse of SimpleRatios based on SafeInt and SafeUInt:

julia> -SimpleRatio{SafeInt}(1,5)
SimpleRatio{SafeInt64}(-1, 5)

julia> -SimpleRatio{SafeUInt}(1,5)
ERROR: OverflowError: cannot negate unsigned number
Stacktrace:
 [1] -(x::SimpleRatio{SafeUInt64})
   @ Ratios ~\.julia\dev\Ratios\src\Ratios.jl:78
...

After the addition of these methods integer overflow can be detected in Interpolations.jl as in JuliaMath/Interpolations.jl#457

julia> using Interpolations

julia> itpi = LinearInterpolation(SafeInt[1,10000],SafeInt[1,10000]; extrapolation_bc=Line())
2-element extrapolate(interpolate((::Vector{SafeInt64},), ::Vector{SafeInt64}, Gridded(Linear())), Line()) with element type Float64:
 SimpleRatio{SafeInt64}(99980001, 99980001)
 SimpleRatio{SafeInt64}(999800010000, 99980001)

julia> itpi(10001)
ERROR: OverflowError: 999800010000 * 99980001 overflowed for type Int64
Stacktrace:
 [1] throw_overflowerr_binaryop(op::Symbol, x::Int64, y::Int64)
   @ Base.Checked .\checked.jl:154
 [2] checked_mul
   @ .\checked.jl:288 [inlined]
 [3] *
   @ ~\.julia\packages\SaferIntegers\zbJSp\src\arith_ops.jl:21 [inlined]
 [4] +
   @ ~\.julia\dev\Ratios\src\Ratios.jl:35 [inlined]
 [5] extrapolate_axis
   @ ~\.julia\dev\Interpolations\src\extrapolation\extrapolation.jl:168 [inlined]
 [6] extrapolate_value
   @ ~\.julia\dev\Interpolations\src\extrapolation\extrapolation.jl:162 [inlined]
 [7] (::Interpolations.Extrapolation{Float64, 1, Interpolations.GriddedInterpolation{Float64, 1, SafeInt64, Gridded{Linear{Throw{OnGrid}}}, Tuple{Vector{SafeInt64}}}, Gridded{Linear{Throw{OnGrid}}}, Line{Nothing}})(x::Int64)
   @ Interpolations ~\.julia\dev\Interpolations\src\extrapolation\extrapolation.jl:52
 [8] top-level scope
   @ REPL[6]:1

Additionally a test suite is added to test the integration of the two packages.

[JeffreySarnoff]

as an aside JuliaMath/Interpolations.jl#457 (comment)

[mkitti]

In terms of benchmarks, using SaferInt rather than Int results in a ~2.5% increase in time.

julia> f(ipti) = for i=1:10000; itpi(i); end
f (generic function with 2 methods)

julia> itpi = LinearInterpolation(Int[1,10000],Int[1,10000]; extrapolation_bc=Line())
2-element extrapolate(interpolate((::Vector{Int64},), ::Vector{Int64}, Gridded(Linear())), Line()) with element type Float64:
 SimpleRatio{Int64}(99980001, 99980001)
 SimpleRatio{Int64}(999800010000, 99980001)

julia> itpi_safe = LinearInterpolation(SafeInt[1,10000],SafeInt[1,10000]; extrapolation_bc=Line())
2-element extrapolate(interpolate((::Vector{SafeInt64},), ::Vector{SafeInt64}, Gridded(Linear())), Line()) with element type Float64:
 SimpleRatio{SafeInt64}(99980001, 99980001)
 SimpleRatio{SafeInt64}(999800010000, 99980001)

julia> @benchmark $f($itpi)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  256.500 μs …   2.060 ms  ┊ GC (min … max): 0.00% … 85.61%
 Time  (median):     304.500 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   323.403 μs ± 151.025 μs  ┊ GC (mean ± σ):  4.67% ±  8.32%

  ▁██▄                                                          ▂
  █████▇█▆▅▆▅▅▅▇▆▆▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▃▃ █
  256 μs        Histogram: log(frequency) by time       1.68 ms <

 Memory estimate: 460.77 KiB, allocs estimate: 19489.

julia> @benchmark $f($itpi_safe)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  265.200 μs …   2.593 ms  ┊ GC (min … max): 0.00% … 77.90%
 Time  (median):     309.500 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   331.228 μs ± 153.054 μs  ┊ GC (mean ± σ):  4.58% ±  8.29%

  ▃█▇▄   ▁                                                      ▂
  █████▅▆█▆▄▄▅▆▇▆▆▆▅▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▄ █
  265 μs        Histogram: log(frequency) by time       1.66 ms <

 Memory estimate: 460.77 KiB, allocs estimate: 19489.

[JeffreySarnoff]

Running the same benchmarks on 1.8.0-DEV, I see similar results with @benchmark using n=10_000. I see little timing difference with @Btime using n=100_000. and the @benchmark means are about the same, though the medians are like the results for n=10_000.

julia> BenchmarkTools.@benchmark ($f)($itpi_safe)
BenchmarkTools.Trial: 5515 samples with 1 evaluation.
 Range (min … max):  808.500 μs …   2.637 ms  ┊ GC (min … max): 0.00% … 67.27%
 Time  (median):     833.800 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   904.123 μs ± 224.925 μs  ┊ GC (mean ± σ):  3.52% ±  9.37%

  █▇▃▃▃▂▁▁             ▁                                        ▁
  ██████████████▇█▇██▇██▇██▇▆▆█▆▇▆▅▆▄▅▁▃▃▁▄▁▁▁▁▄▃▃▁▁▁▁▁▁▁▅▇▇█▇▆ █
  808 μs        Histogram: log(frequency) by time       2.02 ms <

 Memory estimate: 1.21 MiB, allocs estimate: 48979.

julia>

julia> BenchmarkTools.@benchmark ($f)($itpi)
BenchmarkTools.Trial: 5679 samples with 1 evaluation.
 Range (min … max):  793.500 μs …   3.287 ms  ┊ GC (min … max): 0.00% … 58.88%
 Time  (median):     811.200 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   878.120 μs ± 238.196 μs  ┊ GC (mean ± σ):  4.08% ±  9.88%

  █▆▄▂▂▁▁                                                       ▁
  █████████▇▇▇▇▇▇▇▇▆▆▇▆▆█▆▇▄▅▆▆▅▄▅▃▁▁▃▄▁▃▁▃▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▇█▇▇▆ █
  794 μs        Histogram: log(frequency) by time       2.13 ms <

 Memory estimate: 1.21 MiB, allocs estimate: 48979.


julia> n = 100_000;

julia> f(itpi) = for i=1:n; itpi(i); end;

julia> itpi = LinearInterpolation(Int[1,n],Int[1,n]; extrapolation_bc=Line());

julia> itpi_safe = LinearInterpolation(SafeInt[1,n],SafeInt[1,n]; extrapolation_bc=Line());

BenchmarkTools.@benchmark ($f)($itpi_safe)
BenchmarkTools.Trial: 533 samples with 1 evaluation.
 Range (min … max):  8.373 ms …  13.536 ms  ┊ GC (min … max): 0.00% … 9.38%
 Time  (median):     9.127 ms               ┊ GC (median):    0.00%
 Time  (mean ± σ):   9.379 ms ± 946.213 μs  ┊ GC (mean ± σ):  3.93% ± 5.80%

  █▇▁▄             ▂
  ████▆▅▆▅▄▃▄▄▃▃▃▃▃██▅▅▅▅▅▅▄▄▃▃▃▄▄▂▃▃▂▃▂▂▃▁▂▂▂▂▂▁▁▁▁▂▁▁▂▁▁▁▁▂ ▃
  8.37 ms         Histogram: frequency by time        12.8 ms <

 Memory estimate: 12.19 MiB, allocs estimate: 498979.

BenchmarkTools.@benchmark ($f)($itpi)
BenchmarkTools.Trial: 531 samples with 1 evaluation.
 Range (min … max):  7.922 ms … 14.253 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     9.322 ms              ┊ GC (median):    0.00%
 Time  (mean ± σ):   9.422 ms ±  1.006 ms  ┊ GC (mean ± σ):  3.88% ± 5.86%

  ▇               ▁█▅   ▂
  █▇▆▇▆▆▆▄▄▄▄▄▆▄▃▄███▇▇▇█▆▆▇▇▅▆▆▅▅▃█▄▇▄▄▃▄▄▃▃▃▃▂▃▄▃▃▃▃▃▁▁▂▁▃ ▄
  7.92 ms        Histogram: frequency by time          12 ms <

 Memory estimate: 12.19 MiB, allocs estimate: 498979.

Julia Version 1.8.0-DEV.459
Commit 3aada5982c (2021-09-01 19:46 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Xeon(R) E-2176M  CPU @ 2.70GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-12.0.1 (ORCJIT, skylake)
Environment:
  JULIA_NUM_THREADS = 6

[JeffreySarnoff]

fwiw, that using SafeInt64s only requires 1.025 the time for Int64s is quite good
considering the return on time invested (an extra 1.5 seconds each minute).

[timholy]

Color me impressed: I did not realize that checked arithmetic now had such low overhead. Nice work on SaferIntegers, @JeffreySarnoff.

Will this also apply for @simd- and @avx-annotated @inbounds loops? I think both should be composable with Interpolations, but unfortunately anything with a branch in it usually forces turning off vectorization. The current unsafe integers operations should be fine, but the ones in SaferIntegers may not be. Conversely, #19 (comment) should compile down to a select statement which should compatible with vectorization. (select is the equivalent of ifelse and does not contain a true branch. But Julia's compiler is usually smart enough to translate an if/else or ternary operation into select if it's applicable, so often you don't need to use ifelse explicitly anymore.)

[timholy]

(Note that my comment applies only to interpolation, not extrapolation, as extrapolation already comes with a branch. So this should be benchmarked using the lower-level API of interpolations.)

[mkitti]

I'm leaning towards closing this in favor of JeffreySarnoff/SaferIntegers.jl#20.

[timholy]

I agree, but let's let that be merged first. It doesn't seem that CI is set up on that repo, so there could be unintended consequences (unless you tested locally).

[mkitti]

I tested locally. The most difficult aspect was getting type promotion to work again in the same manner since dispatch was modified.

[JeffreySarnoff]

looking ..

[JeffreySarnoff]

I have requested two changes JeffreySarnoff/SaferIntegers.jl#20 (review)
JeffreySarnoff/SaferIntegers.jl#20 (review)
once done (or, should you convince me otherwise) I will merge the PR

[JeffreySarnoff]

The modification to SaferIntegers.jl is merged. This change requires a new major version imo, so now we have v3 -- I expect that will require extra days in the package management merging.

[JeffreySarnoff]

Thank you @mkitti for the perspicacious shepherding.