Commit
2 years ago
gh-37595: Simplify computation of all points for curves over finite fields This PR removes the call to `_points_fast_sqrt()` for computing the set of all points as it (seems) to always be slower than using `_points_via_group_structure()`. While performing this change, I also attempted to clean up the code and simplify / clarify certain docstrings. ## Justification When a curve is defined over a prime order finite field with order larger than 50, the set of all points is computed using the group structure of the curve. However, when a curve is implemented over a non-prime field or a field with order less than 50, the set of all points is instead computed by brute-force enumeration by checking all x-coordinates. For all cases I have tested, this enumeration is slower. ### Small characteristic, prime field When `p < 50`: ```py sage: E = EllipticCurve(GF(11), [1,1]) sage: %timeit E._points_via_group_structure() 165 µs ± 8.19 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each) sage: %timeit E._points_fast_sqrt() 366 µs ± 34.2 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each) sage: sorted(E._points_via_group_structure()) == sorted(E._points_fast_sqrt()) True ``` When `p > 50` ```py sage: E = EllipticCurve(GF(163), [1,1]) sage: %timeit E._points_via_group_structure() 2.49 ms ± 40.6 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) sage: %timeit E._points_fast_sqrt() 4.1 ms ± 112 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) sage: sorted(E._points_via_group_structure()) == sorted(E._points_fast_sqrt()) True ``` ### Small characteristic, non-prime field When the order is smaller than 50: ```py sage: E = EllipticCurve(GF(2^5), [1,0,1,0,1]) sage: %timeit E._points_via_group_structure() 277 µs ± 10.2 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each) sage: %timeit E._points_fast_sqrt() 2.96 ms ± 135 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) sage: sorted(E._points_via_group_structure()) == sorted(E._points_fast_sqrt()) True ``` When the order is larger than 50: ```py sage: E = EllipticCurve(GF(11^3), [1,1]) sage: %timeit E._points_via_group_structure() 15.6 ms ± 1.17 ms per loop (mean ± std. dev. of 7 runs, 100 loops each) sage: %timeit E._points_fast_sqrt() 71.4 ms ± 2.94 ms per loop (mean ± std. dev. of 7 runs, 10 loops each) sage: sorted(E._points_via_group_structure()) == sorted(E._points_fast_sqrt()) True ``` ## Further comments - The commit which introduced `_points_fast_sqrt()` as a method implemented it for hyperelliptic curves over finite fields and then added `HyperellipticCurve_finite_field` as a parent to `EllipticCurve_finite_field` to inherit this function. With the removal of the slower `_points_fast_sqrt()`, this parent inheritance has been removed. - `ruff` noticed that `from sage.sets.set import Set` was unused, so I removed it ### :memo: Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. URL: https://github.com/sagemath/sage/pull/37595 Reported by: Giacomo Pope Reviewer(s):
Author
Release Manager
Loading