num-integer
num-integer copied to clipboard
Implement more number theory functions
This implements the extended Euclidean algorithm, the modular inverse and modular exponentiation.
- This is taken from https://github.com/vks/discrete-log and uses the traits from
num-traits
. - To avoid break backwards compatibility, everything was added as a free function. However, for example
powm
should be part of a trait, because bigint libraries can implement it more efficiently. - The trait requirements for
powm
are not pretty, because the existing traits were not enough.
Fixes #3.
Sorry I haven't reviewed this -- I'm mostly pondering what the public API should look like. I think you're probably right that powm
should be a trait, and maybe the others too, but I don't want to get too fine-grained about this. I'm also not fond of public fields as in GcdResult
.
I'm a bit concerned that the trait bounds are leaking implementation details. If we change the implementation and require different traits, it can easily become a breaking change.
I'm also not fond of public fields as in GcdResult.
Why? It is similar to returning a tuple, except that the fields are named.
You only need the trait bounds for a generic or default impl. The alternative is to only macro-impl the primitives here, then implement bigint separately, and leave others to their own. That's not necessarily the greatest, but how many other Integer
types are there anyway?
I fought with this for Roots
-- I started with a default impl, but I wasn't happy with the performance I could get generically. (That's a bit of an indictment against generic num
, I know...)
My basic objection to public fields is that it makes it hard to make changes to the struct. You're right that it's an improvement over a plain tuple though, which also couldn't change. The more flexible path is an opaque struct with accessors, but then you can't easily unpack values.
The alternative is to only macro-impl the primitives here, then implement bigint separately, and leave others to their own. That's not necessarily the greatest, but how many other Integer types are there anyway?
This would work. The other integer types mostly implement it anyway. What should the traits look like? Putting several functions into one trait makes it hard to extend, having one trait per function is probably inconvenient.
My basic objection to public fields is that it makes it hard to make changes to the struct. You're right that it's an improvement over a plain tuple though, which also couldn't change. The more flexible path is an opaque struct with accessors, but then you can't easily unpack values.
In this case, the only possible change I can think of is returning (a / gcd(a, b), b / gcd(a, b))
in the struct as well. The field names are unlikely to change, because their names don't really matter.
We could add a private field:
pub struct GcdResult<T> {
/// Greatest common divisor.
pub gcd: T,
/// Coefficients such that: gcd(a, b) = c1*a + c2*b
pub c1: T, pub c2: T,
seal: ()
}
It would allow adding fields in future.
Is there any hope for this getting merged? Would love to do modular inverses on bigint.
I rebased on master and made GcdResult
extendable as suggested by @strake.
@cuviper Should I move the added functions to a new NumberTheory
trait?
Extended GCD was already merged in #19, so this should now build on top of that if possible.
Should I move the added functions to a new
NumberTheory
trait?
Can it just be added to Integer
? The new methods can have additional where
constraints if needed for the default impl.
Extended GCD was already merged in #19, so this should now build on top of that if possible.
My implementation is not equivalent: It does not use Clone
, but relies on NumRef
and RefNum
instead. It also consumes the input, instead of taking it by reference and forcing a clone.
The implementation currently part of Integer
has some possibly redundant allocations, which are unavoidable without changing the signature of the method.
I don't know which of the two signatures is preferable. The one on master with Clone
, or the one in this PR with NumRef
and RefNum
(which would be a breaking change)? This also affects the other functions (normalize
, inverse
and powm
), which in their current implementation (in this PR) use NumRef
and RefNum
to avoid allocations.
@cuviper Which option do you prefer?
- Add
NumRef
andRefNum
to the relevantInteger
methods (avoiding unnecessary allocations but requiring a breaking change). - Rewrite the new methods to be compatible with the existing interface, by introducing avoidable allocations, but without breaking changes.
FWIW, there's a 'num-modular` crate that have full support for modular operations.