What's the problem
If you run:
sign(pi)
1.0
Now that's a weird error, if we inspect the definition:
using InteractiveUtils
@which sign(pi)
sign(x::AbstractIrrational)
@ Base irrationals.jl:160
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.