JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.77k stars 5.49k forks source link

Rational multiplication 0 * Inf results in DivideError #39638

Open mbravenboer opened 3 years ago

mbravenboer commented 3 years ago
julia> x = Rational{Int128}(0,1)
0//1

julia> y = Rational{Int128}(1,0)
1//0

julia> x == 0
true

julia> y == Inf
true

julia> x * y
ERROR: DivideError: integer division error
Stacktrace:
 [1] div at ./int.jl:909 [inlined]
 [2] divgcd at ./rational.jl:44 [inlined]
 [3] *(::Rational{Int128}, ::Rational{Int128}) at ./rational.jl:308
 [4] top-level scope at REPL[26]:1

The problem is that the gcd is 0 with x.num and y.den both being 0

function *(x::Rational, y::Rational)
    xn, yd = divgcd(promote(x.num, y.den)...)
    xd, yn = divgcd(promote(x.den, y.num)...)
    unsafe_rational(checked_mul(xn, yn), checked_mul(xd, yd))
end

function divgcd(x::Integer,y::Integer)
    g = gcd(x,y)
    div(x,g), div(y,g)
end

For any x, I think an appropriate value for 0 * x would be 0?

simeonschaub commented 3 years ago

This is definitely intentional, although it might deserve a better error message. Note that the corresponding floating point computation 0.0 * Inf also returns NaN, not 0.0. We could technically allow 0 // 0 as the equivalent of NaN for Rational, but silent NaNs often cause more problems than they solve, so I don't really think we want to do this.

mbravenboer commented 3 years ago

Thanks for the background @simeonschaub .

We were generally hoping that the Rational type would have more of the desirable mathematical properties, like associativity, 0 * x = x etc (modulo overflow). If that is a goal of the Rational type (maybe it is not), then the binary floating-point standard is maybe not a good reference?

simeonschaub commented 3 years ago

That's generally not something that's possible in a consistent way if you decide to include +/- infinity as well. In practice, I would think that you can often just ignore the existence of these infinities or explicitly check against them.

timholy commented 3 years ago

0 * Inf = 0 definitely seems like bad math. Consider $\lim{x\rightarrow 0} x/x$; the answer is of course 1, not zero. But of course $\lim{x\rightarrow 0} 2x/x = 2$. So it's not an operation you can define meaningfully, so either the error or a NaN-equivalent is perfect.

mbravenboer commented 3 years ago

I see, yeah I realize that in our system maybe the problem is that infinity should not have been allowed to begin with. The Rational type chooses to support infinity as a special value, but I probably didn't want that. The mathematical properties I had in mind seem okay if you exclude infinity.