-
Notifications
You must be signed in to change notification settings - Fork 71
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
remove setrounding from functions implementation #442
Comments
At least for
I think it could just be
or is there something I am missing? |
for |
Also, looking at the current implementation of fma, I don't quite get why
I think if any of the inputs is |
@JeffreySarnoff: Do you have any thoughts about directed rounding for |
I like it. |
I need to play with fma and see if/how there is helpful, guidable meshing with [one of] our approaches to rounding. |
step 0: we need Julia to intervene, restoring our ability to choose, to set, to get IEEE floating point rounding modes so please alert those who may think this is has been resolved -- let them know what you know of things now in the way You can obtain for Absent that, one may use
|
(which approach is more performant depends on how performant or otherwise setting and resetting rounding happens to be, |
The issue with On the bright side, I tried the alternative proposed by JeffreySarnoff and it seems to work 🎉 . I also did a small benchmarking with 1.5, where the original version with
I don't have the necessary skills or knowledge to comment about pros and cons of using setrounding, but I follow the discussion with interest. |
after the PR in |
Where is the current implementation type unstable? |
the function min_ignore_nans (sorry I misspelled this in all previous comments) seems to be type unstable
while on v1.5
I think the instability is caused by the splatting inside
|
as a start, these more structured variants are type stable
|
Great to have a fix for the type instability!
Could we simply check if some of the inputs are |
if any is NaN the result is (NaN, NaN)
|
|
Now that
this is a slightly modified version of the current one, using normal min and max and checking for NaI at the beginning of the function.¨ Not sure how this compares performancewise to the manual directed rounding, I can try to craft a benchmark on my machine. |
I am putting together some comparisons too.
I had expected
|
The
|
why does
show |
It seems the display function automatically rounds down the lower bound and rounds up the upper bound to the chosen number of digits (6 by default), regardless of whether the lower and upper bound are the same https://github.com/JuliaIntervals/IntervalArithmetic.jl/blob/master/src/display.jl#L190-L191 |
I think we should avoid setrounding completely. Yes we should probably fix the display to it doesn't round if that's not necessary, if it's possible to do so. |
There is code in The code here is not trying to do that, so it should not round strings. Using |
I set up a small benchmark candidates are fma with setrounding
and fma1 using manual directed rounding and the helping functions as defined above
and the benchmarking
so fma1 is also faster in addition to not using setrounding |
Wow, that's really much faster. That's why pretty convincing! |
15x is 10 times faster than 1.5x, which is my waterline for "worth the effort" |
The version with manual directed rounded apparently fails for "half unbounded intervals", e.g.
Incidentally, now filtering out the NaN before taking the maximum/minimum fixes the problem (in the sense that all tests pass) 😃 . I replaced
so still faster than the original one but quite a loss in efficiency. I think the problem is that |
Why not do something similar to
When I have used this pattern in the past, I wrote a fast binary-based |
Ok I think I figured out how to avoid filtering NaNs. Here is a running example to explain what I mean. The problem is that if we want to compute e.g.
and the first two poison the computation of the maximum and hence the need for max/min_ignore_nans. Followinf Jeffrey's suggestions I modified the function
i.e. if the result of the scalar fma is NaN, it returns -Inf as upper bound and Inf as lower bound. This way those are automatically filtered out when taking the maximum of upper bounds and the minimum of lower bounds, because the only case in which Summa summarum, now all tests pass (the ITF1788 test suite has at least 108 tests checking those half unbounded intervals cases, so I guess they are quite exhaustive), no need for filtering NaN out and the benchmark is
as a final small side note, at the beginning of
to
squeezing a little more efficiency, now the benchmark is
I will soon polish the code and push it to the PR of this issue ( #443 ), comments, feedback and improvement suggestions are always welcome of course. |
With the benchmarking I did this morning
is sometimes less performant than each of these
(it goes so quickly that when none are NaN, the one-off timings are indistinguishable) I displayed |
cool, I did not think it that way. Thanks for the insight! I just benchmarked the routines, I do not have any benchmarks where those routines are part of a bigger real-world code. I switched back to |
Fixed by PR #593. |
Some functions do not work on 1.6 on windows. Particularly, I found the functions
abs, mig, fma, sqrt
. The core problem seems that they usesetrounding
.abs
callsmig
andmig
andfma
use directlysetrounding
(source code here ).sqrt
I am not sure, this code does use set rounding, but the other functions defined there (inv, tanh etc.) do not fail. Here's the full stacktrace for referenceThe text was updated successfully, but these errors were encountered: