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

add RoundingMode argument to convert #8845

Closed
wants to merge 3 commits into from

Conversation

simonbyrne
Copy link
Contributor

Adds an extra RoundingMode argument to some convert calls. This allows things like

julia> Float64(pi,RoundDown)
3.141592653589793

julia> Float64(pi,RoundUp)
3.1415926535897936

This uses stagedfunctions internally, so reduce to constants at runtime. I've also tried to improve the docs a bit.

It occurs to me that we could also use also stagedfunctions to get rid of the "value" argument in the @math_const macro.

cc: @nolta

@simonbyrne
Copy link
Contributor Author

It turns out to be significantly quicker to just do an explicit check than actually play around with rounding modes (about 20x for Float64 -> Float32).

@ViralBShah
Copy link
Member

Love this.

@nolta
Copy link
Member

nolta commented Oct 29, 2014

Great!

Could we add these methods to call instead of convert? I worry that if we start adding arguments to convert, there's no telling where that will end.

Why are the get/set_rounding_raw functions needed?

@simonbyrne
Copy link
Contributor Author

The get/set_rounding_raw functions slightly speed up with_rounding by making its internals type-stable (I was using it in the first version of the Float64 -> Float32 methods). I figured I might as well keep it around.

I'll look at moving it from convert: I haven't played around with the call methods yet, so I'll see how it plays (the examples above "just worked").

That said, it did occur to me that this is actually much more general: once we have correct comparisons, the code I've used in the Float64 -> Float32 should also work for any Real -> FloatingPoint, e.g. Rational -> Float64.

@JeffBezanson
Copy link
Member

This is nice.

@simonbyrne
Copy link
Contributor Author

Okay, I've moved the rounding versions from convert to call. I wasn't sure where to put this in the docs, so it's currently undocumented.

@ViralBShah
Copy link
Member

@nolta nolta closed this in 3fe2da2 Nov 4, 2014
@nolta
Copy link
Member

nolta commented Nov 4, 2014

This didn't merge cleanly anymore, so i rebased and added some docs.

@simonbyrne
Copy link
Contributor Author

Thanks for doing that, I didn't get time to look at it over the weekend.

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 this pull request may close these issues.

4 participants