Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sign(pi) errors #37931

Closed
Moelf opened this issue Oct 7, 2020 · 16 comments · Fixed by #37949
Closed

sign(pi) errors #37931

Moelf opened this issue Oct 7, 2020 · 16 comments · Fixed by #37949

Comments

@Moelf
Copy link
Contributor

Moelf commented Oct 7, 2020

Didn't find an existing issue

julia> sign(pi)
ERROR: InexactError: Bool(-1)

https://discourse.julialang.org/t/problem-with-sign-pi/47801/

@JeffBezanson
Copy link
Member

What type should it return? Float64?

@Moelf
Copy link
Contributor Author

Moelf commented Oct 7, 2020

that's the real question. I would say Float64 since if one is using a Real number probably they are not dealing with integers in the rest of the expression anyways. (reason why 3 * pi gives Float64)

or Int8 for minimal promotion. But that can be weird at first glance. (but at the same time, one(pi) = true already....)

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Oct 7, 2020

Can you fix this with:

julia> import Base.sign

julia> sign(:Irrational{:π}) = true

i.e. why not with Bool?

I tried to fix for all (only two?) such constants, and all future ones:

julia> Revise.track(Base)

julia> sign(AbstractIrrational) = true
┌ Error: Failed to revise /home/pharaldsson_sym/julia-1.6-a0a68a54d6/share/julia/base/Base.jl

without the Revise command it ran, while ineffective, since Real is checked first? It there no way for it to be checked later?

@Moelf
Copy link
Contributor Author

Moelf commented Oct 7, 2020

@PallHaraldsson you mean:

julia> sign(::AbstractIrrational) = true
sign (generic function with 10 methods)

There are a few of them.

yeah, that could work actually, since there's no way to represent a negative Irrational in the first place (they are just Symbols under the hood)

@simeonschaub
Copy link
Member

simeonschaub commented Oct 7, 2020

What type should it return? Float64?

Since signed(true) returns an Int, that seems like the most intuitive answer to me.

@Moelf AbstractIrrationals are also defined by packages like Tau.jl, so I don't think it's safe to assume that an AbstractIrrational is always positive.

@Moelf
Copy link
Contributor Author

Moelf commented Oct 7, 2020

@simeonschaub I see. that's true, but in terms of Julia's representation, we can't really represent a negative irrational number. -irr will send it to Float64.

@simeonschaub
Copy link
Member

I am not sure I follow. For example, you can do the following:

julia> Base.@irrational negpi -3.141592653589793 -big(pi)

julia> negpi
negpi = -3.141592653589...

@Moelf
Copy link
Contributor Author

Moelf commented Oct 7, 2020

I'm saying that:

julia> dump(-3//5)
Rational{Int64}
  num: Int64 -3
  den: Int64 5

the sign is encoded in num, but the irrational number are "just" symbols. But yes, I agree that hack fix may introduce hidden bugs. So an all-around approach is to take the sign of the Float64 representation of that irrational number.

@petvana
Copy link
Member

petvana commented Oct 8, 2020

As a user, I would expect that the following test is satisfied for any type (except for unsigned ones, obviously).

@test sign(pi) === -sign(-pi)

(Otherwise, I may cause unexpected type instability.)

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Oct 9, 2020

Is this also bad? I'm just not sure:

julia> one(pi) === one(-pi)  # and similarily for zero(pi) === zero(-pi)  and sign(pi) === one(pi)
false

@Moelf
Copy link
Contributor Author

Moelf commented Oct 9, 2020

more reason to make everything about irrational just Float64 because of:

-(x::AbstractIrrational) = -Float64(x)

@petvana
Copy link
Member

petvana commented Oct 10, 2020

This is also counterintuitive.

julia> pi == one(pi) * pi
false

julia> zero(pi) == one(pi) * pi - pi
true

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Oct 10, 2020

The former is probably by design. Note:

julia> pi ≈ one(pi)*pi
true

julia> big(1.0)*pi
3.141592653589793238462643383279502884197169399375105820974944592307816406286198

julia> big(1.0)*pi == pi  # only in some CAS where 1.0 isn't Float converting pi to any finite Float, would this be the same.
false

@petvana
Copy link
Member

petvana commented Oct 10, 2020

The former is probably by design. Note:

I have found in docs of one()

Return a multiplicative identity for `x`: a value such that
`one(x)*x == x*one(x) == x`.

So I have opened a separate issue for that.

@StefanKarpinski
Copy link
Member

The only solutions to most of these complaints are to delete the Irrational type entirely or to turn it into a full blown symbolic system. These things are only meant to allow you to use pi with different precisions easily, not to do computer algebra.

@oscardssmith
Copy link
Member

So what you're saying is that the only 1.0 compatible fix is adding a basic CAS? Should good to me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants