-
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
Fix thread-safe in rounding #615
Conversation
At a quick look this looks good! One question though, the CRlib methods are defined for Regarding the comparison with irrationals. It's not only not thread-safe, but also not always correct. The implementation of rounding to float for irrationals in base doesn't guarantee faithful rounding. I recently noticed this and though I would open an issue, but haven't gotten to it yet. Here is an example, defining the completely useless irrational number julia> Base.@irrational qq (1 - big(π)^-300)
julia> interval(qq, qq)
[1.0, 1.0]_com Clearly the lower bound for the interval should be less than 1. The issue is that julia> qq < 1
false which is clearly not correct. |
Thx for taking a look. In these situations, Oh what a pity that comparisons to irrationals don't work 😞. So we need to rework our |
It will work in practice for all irrationals defined in Base at least (except for not being thread-safe). The only issue is for irrationals that are extremely close to floating point numbers, so that 256 bits of precision is not enough to correctly round them. |
But they call CRlibm, so are not using |
Oh I see the lines you are referring too. Yes, that probably should be |
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #615 +/- ##
===========================================
+ Coverage 0 83.64% +83.64%
===========================================
Files 0 25 +25
Lines 0 2140 +2140
===========================================
+ Hits 0 1790 +1790
- Misses 0 350 +350 ☔ View full report in Codecov by Sentry. |
This PR does not rework the quadrant method defined used for trigonometric functions which is not thread-safe. Fixes #612. |
This PR fixes the rounding mechanism to be thread-safe. It removes CRlibm.jl as a dependency, and directly calls methods from CRlibm_jll, or from MPFR as suggested by @Joel-Dahne.
This PR also fixes rounding for intervals with rational bounds by returning a float interval for non exact operations.
To do: we should probably also fix our quadrant method for trigonometric functions which is not thread-safe due to JuliaLang/julia#52862