Adventure into sign of irrational number


What's the problem

If you run:

sign(pi)
InexactError: Bool(-1)

Now that's a weird error, if we inspect the definition:

using InteractiveUtils
@which sign(pi)

sign(x::Real) in Base at number.jl:118
which says:

sign(x::Real) = ifelse(x < zero(x), oftype(one(x),-1), ifelse(x > zero(x), one(x), typeof(one(x))(x)))

The problems happens at oftype(one(x),-1), usually this gives you the "-1" of the type of first argument:

@show oftype(1, -1)
@show oftype(1.0, -1)
@show oftype(1f0, -1)
oftype(1, -1) = -1
oftype(1.0, -1) = -1.0
oftype(1.0f0, -1) = -1.0f0

Unity to blame

The issue for this specific implementation is that one(pi) == true. This is because we don't want to accidentally 'promote' user's expression type if they ever have something like float * one(pi).

This is the same reason why we have AbstractIrrational in the first place:

@show typeof(1.0*pi)
@show typeof(Float32(1)*pi)
@show typeof(Float16(1)*pi)

typeof(1.0pi) = Float64
typeof(Float32(1) * pi) = Float32
typeof(Float16(1) * pi) = Float16
For example if user is running something on the GPU, you don't want to silently make everything Float64 if user used pi in their code.

Solution?

One can argue that sign(::Real) is actually too general, especially because there's no way to represent "negative irrational" in the first place. This is obvious if we dump what's inside an irrational number:

@show Union{Rational, AbstractIrrational} <: Real
dump(-3//5)

Union{Rational, AbstractIrrational} <: Real = true
Rational{Int64}
  num: Int64 -3
  den: Int64 5
where the "sign" is encoded in the numerator, compare to:

dump(pi)

Irrational{:π} π
where the information is simply encoded in the symbol of that constant.

Looking at the code base and actually try to think about what are the important mathematical constants, it appears to me that there's no "naturally negative" irrational math constants anyway.

Base.@irrational π        3.14159265358979323846  pi
Base.@irrational ℯ        2.71828182845904523536  exp(big(1))
Base.@irrational γ        0.57721566490153286061  euler
Base.@irrational φ        1.61803398874989484820  (1+sqrt(big(5)))/2
Base.@irrational catalan  0.91596559417721901505  catalan

So a hacky fix would be:

sign(::AbstractIrrational) = true

(also exploiting the fact that currently there's no such method defined (which is why sign(pi) went to the ::Real method.)

But since other packages can freely extend AbstractIrrational, this can introduce fatal bugs.... as pointed out by people.