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

Adding rounding functions for Fixed #153

Closed
kimikage opened this issue Dec 24, 2019 · 3 comments · Fixed by #158
Closed

Adding rounding functions for Fixed #153

kimikage opened this issue Dec 24, 2019 · 3 comments · Fixed by #158

Comments

@kimikage
Copy link
Collaborator

kimikage commented Dec 24, 2019

As I mentioned in #139, there are no rounding functions for Fixed.

julia> round(0.502N0f8)
1.0N0f8

julia> round(0.500Q0f7)
ERROR: MethodError: no method matching round(::Fixed{Int8,7}, ::RoundingMode{:Nearest})

An important difference between Normed and Fixed is that Normed has no tie (or half) numbers, but Fixed has.
So, I also think it is a good idea to support the 2nd argument (i.e. RoundingMode) explicitly.

@kimikage
Copy link
Collaborator Author

kimikage commented Dec 24, 2019

BTW, I think the current implementation has an overflow problem. (1 is a Int64 or Int32 number.)

function round(x::Normed{T,f}) where {T,f}
mask = convert(T, 1<<(f-1))
y = trunc(x)
return convert(T, reinterpret(x)-reinterpret(y)) & mask>0 ?
Normed{T,f}(y+oneunit(Normed{T,f})) : y
end

The method using divrem proposed in the PR #143 will probably solve the overflow problem. (I guess it cannot handle the signed Normed correctly, though.)

function round(x::Normed{T,f}) where {T,f}
    d, r = divrem(reinterpret(x), rawone(x))
    return Normed{T,f}((d + r>>(f-1))*rawone(x), 0)
end

@kimikage
Copy link
Collaborator Author

kimikage commented Dec 25, 2019

julia> round(typemax(N0f64)) # on 64-bit version
ERROR: InexactError: check_top_bit(Int64, -9223372036854775808)

julia> ceil(typemax(N1f7)) # wraparound
0.984N1f7

testtrunc is not so powerful even though it spends a lot of time.

@kimikage
Copy link
Collaborator Author

This is off-topic, but not only rem but also div is going to accept a rounding mode in Julia v1.4.

timholy added a commit that referenced this issue Jan 11, 2020
Add rounding functions for `Fixed` (Fixes #153)
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.

1 participant