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

Broken integer rounding for big(Inf) #44662

Closed
Keno opened this issue Mar 17, 2022 · 5 comments
Closed

Broken integer rounding for big(Inf) #44662

Keno opened this issue Mar 17, 2022 · 5 comments
Labels
bug Indicates an unexpected problem or unintended behavior maths Mathematical functions

Comments

@Keno
Copy link
Member

Keno commented Mar 17, 2022

julia> round(Integer, Inf)
ERROR: InexactError: trunc(Int64, Inf)
Stacktrace:
 [1] trunc
   @ ./float.jl:781 [inlined]
 [2] trunc
   @ ./float.jl:354 [inlined]
 [3] round(#unused#::Type{Integer}, x::Float64)
   @ Base ./float.jl:359
 [4] top-level scope
   @ REPL[14]:1

julia> round(Integer, big(Inf))
0
@ViralBShah ViralBShah added the maths Mathematical functions label Mar 17, 2022
@oscardssmith oscardssmith added the bug Indicates an unexpected problem or unintended behavior label Mar 18, 2022
@barucden
Copy link
Contributor

Also:

julia> round(Integer, NaN)
ERROR: InexactError: trunc(Int64, NaN)
Stacktrace:
 [1] trunc
   @ ./float.jl:805 [inlined]
 [2] trunc
   @ ./float.jl:364 [inlined]
 [3] round(#unused#::Type{Integer}, x::Float64)
   @ Base ./float.jl:369
 [4] top-level scope
   @ REPL[16]:1

julia> round(Integer, big(NaN))
0

I think it is due to mpfr.jl#L280 calling mpfr_get_z which returns 0 in case of NaN or Inf:

If op is NaN or an infinity, the erange flag is set, rop is set to 0, and 0 is returned.

Is _unchecked_cast the correct place to check for NaN or Inf?

@Keno
Copy link
Member Author

Keno commented Mar 18, 2022

Presumably we should look at the erange flag after the operation and raise an appropriate error.

@barucden
Copy link
Contributor

It seems that the logic is already there for Union{Signed, Unsigned}. It just does not get called, and this definition for BigInt (added in #33681) is used instead. Can we just remove the specific method for BigInt and keep the one for Union{Signed, Unsigned}?

@Keno
Copy link
Member Author

Keno commented Mar 18, 2022

The other method doesn't quite work for BigInt, because

julia> typemin(BigInt)
ERROR: MethodError: no method matching typemin(::Type{BigInt})

but yes, add the same logic for checking the erange flag.

barucden added a commit to barucden/julia that referenced this issue Mar 18, 2022
It also fixes `round(Integer, big(NaN))`.

Solves JuliaLang#44662
oscardssmith pushed a commit that referenced this issue Mar 21, 2022
It also fixes `round(Integer, big(NaN))`.

Solves #44662
@barucden
Copy link
Contributor

Solved by #44676 (I just messed up the "fixes" keyword in the PR message)

KristofferC pushed a commit that referenced this issue Mar 23, 2022
It also fixes `round(Integer, big(NaN))`.

Solves #44662

(cherry picked from commit ecf3558)
KristofferC pushed a commit that referenced this issue Apr 19, 2022
It also fixes `round(Integer, big(NaN))`.

Solves #44662

(cherry picked from commit ecf3558)
KristofferC pushed a commit that referenced this issue May 16, 2022
It also fixes `round(Integer, big(NaN))`.

Solves #44662

(cherry picked from commit ecf3558)
KristofferC pushed a commit that referenced this issue Dec 21, 2022
It also fixes `round(Integer, big(NaN))`.

Solves #44662

(cherry picked from commit ecf3558)
staticfloat pushed a commit that referenced this issue Dec 23, 2022
It also fixes `round(Integer, big(NaN))`.

Solves #44662

(cherry picked from commit ecf3558)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior maths Mathematical functions
Projects
None yet
Development

No branches or pull requests

4 participants