make EllipticCurvePoint_field inherit from EllipticCurvePoint #37415
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I agree that points over extension fields should be |
|
I looked at how you dealt with this in #33228; I think this is a neat solution and I'm happy to simplify |
|
Eventually we should still make the machinery for points over extension fields work. But I believe this will need some larger adjustments, in particular within some of the isogeny-related code... |
|
Documentation preview for this PR (built with commit 051126d; changes) is ready! 🎉 |
|
Indeed, there is definitely some work to be done. I ran into a bug with point homsets over extension fields: #37427 |
kwankyu
reviewed
May 9, 2024
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
May 15, 2024
…CurvePoint
The method `EllipticCurve_generic.__contains__()` checks for inheritance
from `EllipticCurvePoint`; however, the other classes for points on
elliptic curves do not inherit from this class. This pull request fixes
this. Note that there is some overlap with sagemath#33228.
This change makes `P in E` much faster:
- Over **Q**:
```python
E = EllipticCurve('37a1')
P = E((0, -1))
%timeit P in E
```
Before: `35.3 µs ± 186 ns per loop (mean ± std. dev. of 7 runs, 10,000
loops each)`
After: `324 ns ± 0.535 ns per loop (mean ± std. dev. of 7 runs,
1,000,000 loops each)`
- Over a finite field:
```python
F.<a> = FiniteField((23, 2), modulus='random')
E = EllipticCurve_from_j(F.random_element())
P = E.random_point()
%timeit P in E
```
Before: `36.4 µs ± 542 ns per loop (mean ± std. dev. of 7 runs, 10,000
loops each)`
After: `333 ns ± 5.03 ns per loop (mean ± std. dev. of 7 runs, 1,000,000
loops each)`
URL: sagemath#37415
Reported by: Peter Bruin
Reviewer(s): Kwankyu Lee, Peter Bruin
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
May 18, 2024
…CurvePoint
The method `EllipticCurve_generic.__contains__()` checks for inheritance
from `EllipticCurvePoint`; however, the other classes for points on
elliptic curves do not inherit from this class. This pull request fixes
this. Note that there is some overlap with sagemath#33228.
This change makes `P in E` much faster:
- Over **Q**:
```python
E = EllipticCurve('37a1')
P = E((0, -1))
%timeit P in E
```
Before: `35.3 µs ± 186 ns per loop (mean ± std. dev. of 7 runs, 10,000
loops each)`
After: `324 ns ± 0.535 ns per loop (mean ± std. dev. of 7 runs,
1,000,000 loops each)`
- Over a finite field:
```python
F.<a> = FiniteField((23, 2), modulus='random')
E = EllipticCurve_from_j(F.random_element())
P = E.random_point()
%timeit P in E
```
Before: `36.4 µs ± 542 ns per loop (mean ± std. dev. of 7 runs, 10,000
loops each)`
After: `333 ns ± 5.03 ns per loop (mean ± std. dev. of 7 runs, 1,000,000
loops each)`
URL: sagemath#37415
Reported by: Peter Bruin
Reviewer(s): Kwankyu Lee, Peter Bruin
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
May 18, 2024
…CurvePoint
The method `EllipticCurve_generic.__contains__()` checks for inheritance
from `EllipticCurvePoint`; however, the other classes for points on
elliptic curves do not inherit from this class. This pull request fixes
this. Note that there is some overlap with sagemath#33228.
This change makes `P in E` much faster:
- Over **Q**:
```python
E = EllipticCurve('37a1')
P = E((0, -1))
%timeit P in E
```
Before: `35.3 µs ± 186 ns per loop (mean ± std. dev. of 7 runs, 10,000
loops each)`
After: `324 ns ± 0.535 ns per loop (mean ± std. dev. of 7 runs,
1,000,000 loops each)`
- Over a finite field:
```python
F.<a> = FiniteField((23, 2), modulus='random')
E = EllipticCurve_from_j(F.random_element())
P = E.random_point()
%timeit P in E
```
Before: `36.4 µs ± 542 ns per loop (mean ± std. dev. of 7 runs, 10,000
loops each)`
After: `333 ns ± 5.03 ns per loop (mean ± std. dev. of 7 runs, 1,000,000
loops each)`
URL: sagemath#37415
Reported by: Peter Bruin
Reviewer(s): Kwankyu Lee, Peter Bruin
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The method
EllipticCurve_generic.__contains__()checks for inheritance fromEllipticCurvePoint; however, the other classes for points on elliptic curves do not inherit from this class. This pull request fixes this. Note that there is some overlap with #33228.This change makes
P in Emuch faster:Before:
35.3 µs ± 186 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)After:
324 ns ± 0.535 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)Before:
36.4 µs ± 542 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)After:
333 ns ± 5.03 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)