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

setrounding/round interaction #26801

Closed
jmkuhn opened this issue Apr 13, 2018 · 7 comments
Closed

setrounding/round interaction #26801

jmkuhn opened this issue Apr 13, 2018 · 7 comments

Comments

@jmkuhn
Copy link
Contributor

jmkuhn commented Apr 13, 2018

julia> setrounding(Float64, RoundUp)
0

julia> round(1.2)
2.0

julia> round(Int, 1.2)
2

julia> round(1.2, RoundNearest)
2.0

julia> round(Int, 1.2, RoundNearest)
2

I expect the last 2 to be 1.0/1.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Apr 13, 2018

The rounding mode affects rounding direction in case of ties. If you want to round down, use the floor function instead of the round function.

@jmkuhn
Copy link
Contributor Author

jmkuhn commented Apr 13, 2018

I'm not sure what you mean by ties in this case. We currently have

julia> setrounding(Float64, RoundUp)
0

julia> round(1.2)
2.0

julia> round(1.2, RoundDown)
1.0

julia> round(1.2, RoundNearest)
2.0

It seems inconsistent to honor RoundDown but not RoundNearest in this case.

@simonbyrne
Copy link
Contributor

simonbyrne commented Apr 13, 2018

I know, it's annoying, but there's not much we can do.

The problem is that LLVM doesn't expose a rounding intrinsic which matches round(x, RoundNearest): the closest they have is llvm.rint which depends on the global rounding mode. Presumably at some point they will need one since there is a proposal to introduce roundeven function into C, but until that happens, this seems like the least worst option.

One thing we may want to do is not export setrounding, since it is quite unreliable (as LLVM doesn't really yet support rounding mode control), and just mark it off as experimental. Also I think the whole idea for global rounding control is bad, and that we should use something like Cassette.jl, but that's a matter for another day.

@jmkuhn
Copy link
Contributor Author

jmkuhn commented Apr 13, 2018

I agree that the idea for global rounding control is bad. Since it is exported I was just tying to make it consistent, but without support from llvm I understand your choice.

@vtjnash
Copy link
Member

vtjnash commented Apr 13, 2018

The problem is that LLVM doesn't expose a rounding intrinsic

It was added recently (http://llvm.org/docs/LangRef.html#llvm-experimental-constrained-rint-intrinsic), although support is likely still considered experimental and limited

@simonbyrne
Copy link
Contributor

Not quite: the run time behaviour still depends on the global rounding mode, it just controls the compile time behaviour.

@simonbyrne
Copy link
Contributor

setrounding has now been deprecated: #27166

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

4 participants