Implement algorithm=generic_small and algorithm=hybrid for elliptic curve points #40223
Conversation
user202729
force-pushed
the
ec-point-order
branch
from
f1a925c to
cfa0425
Compare
|
Documentation preview for this PR (built with commit a305a7d; changes) is ready! 🎉 |
user202729
changed the title
| return generic.order_from_bounds(self, None) | ||
| elif algorithm == 'hybrid': | ||
| lb = 1 | ||
| sqrt_ub = 32 |
There was a problem hiding this comment.
Why not start with 16 instead of 32 just like generic groups?
There was a problem hiding this comment.
No good reason. On the other hand order_from_bounds has (much) higher constant factor than factor(limit=…), so I don't see the harm either.
| if isinstance(N, Integer): | ||
| factorization = N.factor(limit=sqrt_ub) | ||
| if factorization.is_complete_factorization(): | ||
| return self._compute_order(algorithm='pari') |
There was a problem hiding this comment.
Is it possible to save on the number of factorizations performed here? One is being done here and one factorization will be performed in the pari algorithm.
There was a problem hiding this comment.
don't think it's possible. There's #3549 but if trial division works in reasonable time then the number must be very smooth so that the factorization is not a bottleneck.
| if N is None and sqrt_ub >= 5000: | ||
| N = self.curve().order() | ||
| if isinstance(N, Integer): | ||
| factorization = N.factor(limit=sqrt_ub) |
There was a problem hiding this comment.
Can factorization be re-used from previous loop runs?
There was a problem hiding this comment.
technically possible but that probably requires some copy paste and/or strange modification to .factor() to have lower bounds on prime factors. sqrt_ub is increasing geometrically anyway, so the time wasted is not very large (at most linear in the time doing useful work)
| return self._compute_order('pari') | ||
| except PariError: | ||
| pass | ||
| return self._compute_order('generic') |
There was a problem hiding this comment.
I have no idea how can Pari error be raised. Arguably if this line is hit it would be a pari bug.
There was a problem hiding this comment.
What happens if the curve parameters can't be fit into int 64 or int 128 bits? Can Pari work with those too?
There was a problem hiding this comment.
yes:
F = GF(2^70)
E = EllipticCurve([F.random_element() for __ in range(5)])
E.0._compute_order(algorithm='pari')
F = GF(next_prime(2^128))
E = EllipticCurve([F.random_element() for __ in range(5)])
E.0._compute_order(algorithm='pari')
R.<x> = QQ[]
F.<a> = NumberField(x^2+next_prime(2^128))
E = EllipticCurve([F.random_element() for __ in range(5)])
E.0._compute_order(algorithm='pari') # takes forever but it probably won't error out
The main purpose of this is to be able to do the following (from doctest)
Also fix some minor issues.
It might be preferable to upstream this to pari as well.
📝 Checklist
⌛ Dependencies