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

bug in BigFloat trunc / floor / ceil / round #24041

Closed
getzdan opened this issue Oct 7, 2017 · 6 comments
Closed

bug in BigFloat trunc / floor / ceil / round #24041

getzdan opened this issue Oct 7, 2017 · 6 comments

Comments

@getzdan
Copy link
Contributor

getzdan commented Oct 7, 2017

The following error should not occur, as trunc(255.5) is 255 which is a legal UInt8:

julia> trunc(UInt8,BigFloat(255.5))
ERROR: InexactError: trunc(UInt8, 2.555000000000000000000000000000000000000000000000000000000000000000000000000000e+02)
Stacktrace:
 [1] trunc(::Type{UInt8}, ::BigFloat) at ./mpfr.jl:209

The same logic applies to the other casting functions.

A proper solution should be to perform the round/trunc/floor/ceil in the source type, i.e. BigFloat and then cast to the target type and error if it doesn't fit correctly.

The relevant code is:

julia/base/mpfr.jl

Lines 207 to 223 in c6899ec

function trunc(::Type{T}, x::BigFloat) where T<:Union{Signed,Unsigned}
(typemin(T) <= x <= typemax(T)) || throw(InexactError(:trunc, T, x))
unsafe_cast(T, x, RoundToZero)
end
function floor(::Type{T}, x::BigFloat) where T<:Union{Signed,Unsigned}
(typemin(T) <= x <= typemax(T)) || throw(InexactError(:floor, T, x))
unsafe_cast(T, x, RoundDown)
end
function ceil(::Type{T}, x::BigFloat) where T<:Union{Signed,Unsigned}
(typemin(T) <= x <= typemax(T)) || throw(InexactError(:ceil, T, x))
unsafe_cast(T, x, RoundUp)
end
function round(::Type{T}, x::BigFloat) where T<:Union{Signed,Unsigned}
(typemin(T) <= x <= typemax(T)) || throw(InexactError(:round, T, x))
unsafe_cast(T, x, ROUNDING_MODE[])
end

@StefanKarpinski
Copy link
Sponsor Member

A proper solution should be to perform the round/trunc/floor/ceil in the source type, i.e. BigFloat and then cast to the target type and error if it doesn't fit correctly.

Yes, I do think that would work. The assumption required is that you can correctly do the round/trunc/floor/ceil operation in the original type. That seems like a reasonable requirement. Once the value is correct in the source type, the value can simply be converted and rely on domain checking of convert, so the implementation is quite simple:

$op(::Type{T}, x::Number) = convert(T, $op(x))

@drpn
Copy link

drpn commented Oct 10, 2017

I would like to suggest a solution. Can't we convert BigFloat to AbstractFloat first and then, go for trunc?
Thats even working fine. Thus I want to know its performance too.

@StefanKarpinski
Copy link
Sponsor Member

Can't we convert BigFloat to AbstractFloat first and then, go for trunc?

BigFloat is an implementation of AbstractFloat, so that conversion would do nothing.

@drpn
Copy link

drpn commented Oct 11, 2017

Ok, can you just tell me how can I test if I edit the code which I have cloned in my desktop? I want to try both the suggested one and mine.

@mbauman
Copy link
Sponsor Member

mbauman commented Oct 11, 2017

In simple cases like this you can just re-define the method at the REPL:

julia> @eval Base.MPFR function trunc(::Type{T}, x::BigFloat) where T<:Union{Signed,Unsigned}
            (typemin(T) <= x <= typemax(T)) || throw(InexactError(:trunc, T, x))
            println("NEW IMPLEMENTATION!")
            unsafe_cast(T, x, RoundToZero)
        end
WARNING: Method definition trunc(Type{T<:Union{Signed, Unsigned}}, Base.MPFR.BigFloat) in module MPFR at REPL[13]:2 overwritten at REPL[14]:2.
trunc (generic function with 66 methods)

julia> trunc(Int, big(2.4))
NEW IMPLEMENTATION!
2

@laborg
Copy link
Contributor

laborg commented Feb 8, 2022

This has apparently been handled in #29157

julia> trunc(UInt8,BigFloat(255.5))
0xff

@laborg laborg closed this as completed Feb 8, 2022
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

No branches or pull requests

5 participants