-
-
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
Rational power of BigFloat
should use specialised MPFR function
#55204
Comments
Apparently the code is still in the library |
|
At some point we had an |
(The |
My bad, I see we have the |
That works if the numerator |
Note that |
Would something like this be a good first-pass solution? diff --git a/base/mpfr.jl b/base/mpfr.jl
index ed3ea5937c..7833b65284 100644
--- a/base/mpfr.jl
+++ b/base/mpfr.jl
@@ -733,6 +733,25 @@ function ^(x::BigFloat, y::BigInt)
return z
end
+_rootn(z, x, n::ClongMax) = ccall((:mpfr_rootn_si, libmpfr), Int32, (Ref{BigFloat}, Ref{BigFloat}, Clong, MPFRRoundingMode), z, x, n, rounding_raw(BigFloat))
+_rootn(z, x, n::CulongMax) = ccall((:mpfr_rootn_ui, libmpfr), Int32, (Ref{BigFloat}, Ref{BigFloat}, Culong, MPFRRoundingMode), z, x, n, rounding_raw(BigFloat))
+
+function ^(x::BigFloat, y::Rational{<:Union{ClongMax,CulongMax}})
+ # If `y` is of the form `±1//n`, we can use MPFR's builtin nth root without needing to
+ # convert the `Rational` to a `BigFloat`. The `copysign` is because `mpfr_rootn_*` takes
+ # a negative power to mean `-1//n` but `Rational{<:Signed}` uses the numerator to track
+ # the sign.
+ if isone(abs(numerator(y)))
+ z = BigFloat()
+ _rootn(z, x, copysign(denominator(y), numerator(y)))
+ return z
+ else
+ # TODO: Determine whether there are cases where we should prefer doing something
+ # like `x^(p//q) = rootn(x, q)^p`, which will have accuracy issues for `x` near 0
+ return x^BigFloat(y)
+ end
+end
+
^(x::BigFloat, y::Integer) = typemin(Clong) <= y <= typemax(Clong) ? x^Clong(y) : x^BigInt(y)
^(x::BigFloat, y::Unsigned) = typemin(Culong) <= y <= typemax(Culong) ? x^Culong(y) : x^BigInt(y)
|
Actually thinking about this more, as long as |
big(1.3)^(1//7)
currently converts the rational toBigFloat
before computing the power, thereby losing accuracy.It should instead use the specialised MPFR function
mpfr_rootn_si
.cc @oscardssmith
The text was updated successfully, but these errors were encountered: