-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
RFC: Ability to change rounding mode for all floats (cf. #2976) #3149
Conversation
It appears that this should also work on windows. |
The hard coded enums are a bit scary. Is there a way to make it a bit more portable? |
BTW This is really cool stuff! |
Portable in what sense? The values used in |
What I meant is, just like the various system specific constants that are generated in |
Oh, this is great, I had no idea it could be done this way. |
Ok, I'll boot up an Ubuntu virtual machine later to see what is happening, it works fine on both OS X 10.8.3 and Arch Linux. |
Very exciting! Ignorant question: Do we know that LLVM will not reorder code to nullify the effect of changing rounding mode (as per concern earlier expressed @andrioni)? |
I believe it will work if the rounding mode is set by a called function, and not inlined. |
To be safe, does LLVM support some kind of instruction that acts as a barrier to reordering? |
I don't believe so, unless you count function call. There are many LLVM list threads on this topic. |
I'm also a bit surprised that this did work, and it has in all my tests, so I guess the indirection provided by Julia function and/or the ccall is enough to prevent reordering. As using |
Yes, it is well known that indirecting through a function call is sufficient to prevent reordering. |
Minor point: Should we preserve precedent and call one mode RoundTowardZero (rather than RoundToZero)? |
Where's the precedent from? |
My initial impression was created from fenv.h (FE_TONEAREST, FE_UPWARD, FE_DOWNWARD, FE_TOWARDZERO) but that is not definitive. Wikipedia on IEEE 754-1985 shows the rounding modes as: |
I would even be in favor of shorter names: RoundUp, RoundDown, RoundNearest, RoundToZero, RoundFromZero. |
The precedent is clearly varied, and I am content with either Alessandro or Stefan's choice. The capability is key! |
I prefer @StefanKarpinski 's suggested names. It doesn't seem there is a widely used set of names. Also, this pull request needs to be documented in the manual as well in the standard library before merging, or else nobody will ever know that this amazing functionality exists. |
Bump. @andrioni Can you please rebase? I would prefer to get this merged in and bikeshed a bit later. |
@loladiro @vtjnash What about Windows? |
I believe that openlibm includes |
Bump again. It would be nice to get this included in the 0.2 release if possible. |
OK, I'm back. I'll rebase it in some hours! |
Note that windows does not provide fsetround, you'll have to use http://msdn.microsoft.com/en-gb/library/e9b52ceh.aspx instead. |
I thought mingw would provide these functions, according to @vtjnash. |
I thought they were macros, but I might be wrong, @vtjnash? |
They are declared as functions in mingw's fenv.h |
Ok, sorry for the long delay, I had some bad health issues. I'm quite out of date (a month or three), but I'll dedicate this weekend to put things on schedule again. Is this still considered for 0.2? If so, what is lacking? @ViralBShah |
There we go, I think everything is alright now. |
@andrioni Sorry to hear about your health, but it is great to see you back. |
@vtjnash Do you think we can merge this now, and deal with Windows issues post merging? |
I'll check out whether this works on windows tonight and let you know. |
this appears to be lacking documentation. per steven's excellent suggestion, it must be added before it can be merged. |
@@ -0,0 +1,8 @@ | |||
const JL_FE_UNDERFLOW = 0x0010 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file shouldn't be part of the commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I thought I removed it, but now I see I just added the file to .gitignore
. Done, thanks.
It also includes some updates to some related sections about BigFloats.
I just pushed some documentation for both the manual and the standard library reference, with also some minor changes to some related |
It might be good to move all of the function calls to the Also, it looks like you have a build error. But otherwise, I'm fine with merging this. |
It's hard to believe, but apparently the build error is because of Ubuntu. On 12.04, they moved one of the subheaders, What should we do about it? |
Can we change |
I think we could check whether the preprocessor output parsed correctly, and if not, use the default x86/x64 values. I only found this issue on Ubuntu 12.04 (I didn't test it on other versions, I haven't got any other Ubuntu VMs handy), but not on Debian testing or on Arch Linux, so it might not be that big of an issue. |
If the values from fenv.h aren't found by the C preprocessor, it uses the default ones for x86/x64
The last commit uses some ugly regexes, but managed to solve it. |
Any comments? |
I'm testing locally before merging, but it looks great. |
Ability to change rounding mode for all floats (cf. #2976)
Hip, hip, hooray! One more component of reliable numerics in Julia. |
Yes, this is great. Kahan would/will be pleased. |
This is great, thanks @andrioni . I would prefer to use constants instead of types for the modes, as in:
This way |
Really glad to see this! |
Out of curiosity, what was the reason for having separate ordinary float and bigfloat functions (e.g. |
BigFloats support an extra rounding mode: RoundFromZero. The with_rounding_mode function only accepts rounding modes that work for everything. |
This adds three new functions (
get_rounding
,set_rounding
andwith_rounding
) which allow users to change the rounding direction of floating point arithmetic (both Float32 and Float64). I've also changed the behavior of the old BigFloat rounding functions to agree with this new one, which I find much more intuitive and safe (seetests/rounding.jl
for examples in the case of Float32/Float64).I haven't tested this yet on Windows, and it probably shouldn't work, as it depends on the
fenv.h
header which is only available in C99 or POSIX systems, but I don't know how mingw works..