-
Notifications
You must be signed in to change notification settings - Fork 240
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
gcdCoefficents improvements #3597
base: development
Are you sure you want to change the base?
Conversation
8e191bc
to
236a2ae
Compare
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.
I think the name of French mathematician Bézout needs to appear in this doc :)
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.
Sounds good! Updated
Wraps GMP's mpz_gcdext
This matches the behavior of gcd
Convert to SimpleDoc, add new methods, and provide examples
236a2ae
to
7b7daeb
Compare
I feel like this method should return a sequence instead of a list so that the caller can use parallel assignment, but I'm hesitant to make a breaking change. |
it's only used 3 times in packages, AFAICT, so it's worth a shot. |
Slight breaking change. But this gives us the advantage of using parallel assignment on the output.
A few related changes:
GMP has a built-in extended GCD function, so we use it instead of the current one that's implemented at top-level, improving performance:
Before
After
gcdCoefficients(ZZ, RingElement)
andgcdCoefficients(RingElement, ZZ)
have now been defined, promoting the integer to appropriate ring just likegcd
.The documentation has been updated (converting to SimpleDoc, adding the new methods, and adding some examples).