-
-
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
make inplace Rational{BigInt}
arithmetic from gmplib available
#44566
Conversation
It looks like you did not touch the code paths handling rational infinity, like julia> a = big(1//0);
julia> b = big(0//1);
julia> Base.GMP.MPQ.sub!(b, a);
julia> b
0//1 when I'd expect it to be |
@Liozou thanks for your comment! I fixed inplace for infinities, and added some tests |
Co-authored-by: Lionel Zoubritzky <[email protected]>
Co-authored-by: Lionel Zoubritzky <[email protected]>
Co-authored-by: Lionel Zoubritzky <[email protected]>
Just a very simple benchmark of how much speed up is possible using preallocated for
for
The benchmark codeusing BenchmarkTools
z, x, y = Rational{BigInt}(0, 1), Rational{BigInt}(123456789, 3456789), Rational{BigInt}(2323223, 11991398)
@btime +($x, $y)
@btime *($x, $y)
@btime Base.GMP.MPQ.add!($z, $x, $y)
@btime Base.GMP.MPQ.mul!($z, $x, $y)
@info "###########"
x, y = 12345, 3423437
@btime Base.unsafe_rational(BigInt, $x, $y)
@btime Base.GMP.MPQ.set_si!($z, $x, $y)
x = BigInt(2123434)
@btime Base.GMP.MPQ.set_z!($z, $x)
|
Co-authored-by: Lionel Zoubritzky <[email protected]>
Co-authored-by: Lionel Zoubritzky <[email protected]>
Co-authored-by: Lionel Zoubritzky <[email protected]>
Co-authored-by: Lionel Zoubritzky <[email protected]>
Co-authored-by: Lionel Zoubritzky <[email protected]>
@Liozou thank you a lot for your comments ! |
Do you think the need to store @eval $op(a::$T) = $op!(unsafe_rational(BigInt(), BigInt()), a) more efficient by using |
In the definition of I think one way to completely bypass the issue consists in not using |
There is a more in-depth discussion about this in #9826 |
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.
Thank you for this PR! I think it's good now, and a nice addition.
Hi ! This pr restructures arithmetic operations on
Rational{BigInt}
ingmp.jl
to make inplace GMP for big rationals (add!
,sub!
,mul!
, anddiv!
) available fromBase.GMP.MPQ
module.There it also specializes
cmp
(gmp function described in here https://gmplib.org/manual/Comparing-Rationals asmpz_cmp
), that turns out to be faster than current version in Base.Current version of cmp
with cmp from GMP :