Skip to content
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

invmod(n::BitInteger): efficient native modular inverses #52180

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

StefanKarpinski
Copy link
Sponsor Member

@StefanKarpinski StefanKarpinski commented Nov 15, 2023

Implement algorithm described in https://arxiv.org/pdf/2204.04342.pdf. The algorithm is pleasingly simple and efficient and the generic Julia implementation is also really enjoyable.

@StefanKarpinski StefanKarpinski added the needs news A NEWS entry is required for this change label Nov 15, 2023
base/intfuncs.jl Outdated Show resolved Hide resolved
@oscardssmith
Copy link
Member

I'm not sure this is invmod. I think this method should possibly be invmod(n::T, T) where T for consistency

@StefanKarpinski
Copy link
Sponsor Member Author

StefanKarpinski commented Nov 15, 2023

It's a modular inverse where the modulus is implied by the type. In an alternate history I could see having inv(n::T) mean this, but we've already defined that to mean 1/n which is not this operation.

I'm certainly ok with having an invmod(n::Integer, ::Type{T}) where {T<:Integer} method, but its definition would simply be invmod(n % T). I can add that method to the PR.

@StefanKarpinski StefanKarpinski changed the title invmod(n::BitInteger): efficient 1-arg modular inverses invmod(n::BitInteger): efficient native modular inverses Nov 15, 2023
@oscardssmith
Copy link
Member

my point is that I don't think invmod makes sense as a 1 argument function (because inv mod what?)

@JeffBezanson JeffBezanson added the maths Mathematical functions label Nov 15, 2023
@StefanKarpinski
Copy link
Sponsor Member Author

StefanKarpinski commented Nov 15, 2023

because inv mod what?

Mod by the the modulus implied by type of the argument. It seems reasonable to have an abbreviation for invmod(n, typeof(n)). What other function would you add that method to? We could define a new modinv(n) function for that, but why? This seems like a very natural and convenient place for this functionality.

@oscardssmith
Copy link
Member

fair enough.

@StefanKarpinski StefanKarpinski force-pushed the sk/invmod-native branch 5 times, most recently from fbe8426 to bb1307d Compare November 15, 2023 22:13
@StefanKarpinski StefanKarpinski removed the needs news A NEWS entry is required for this change label Nov 15, 2023
@StefanKarpinski
Copy link
Sponsor Member Author

Failures look unrelated, this is a simple PR, and there's a decent spread of successes, so I'm gonna merge.

@StefanKarpinski StefanKarpinski merged commit 045b6f9 into master Nov 17, 2023
4 of 7 checks passed
@StefanKarpinski StefanKarpinski deleted the sk/invmod-native branch November 17, 2023 16:46
@StefanKarpinski StefanKarpinski mentioned this pull request Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants