Modify tests in ell_point.py to be more generic for Conway database update
#37967
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
Make the tests of `.weil_pairing` and `.tate_pairing` independent of the implementation of `GF(65537^2)`, in view of Conway database update
|
LGTM. Otherwise, either one in sagemath/conway-polynomials#5 (comment) works and avoids removing the tests. |
Amend: Fixed typo
|
Nice, thanks. |
|
Documentation preview for this PR (built with commit 6973ae9; changes) is ready! 🎉 |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
May 11, 2024
…or Conway database update
As part of the Conway polynomials database update, two tests in
`schemes.elliptic_curves.ell_point` caused issues due to their
dependency on the precise implementation of the finite field
`GF(65537^2)`:
- The first test (of `.weil_pairing`) checks whether the Sage
implementation still works for an example of fixed `7282`-torsion points
- The second test (of `.tate_pairing`) checks whether the new
implementation, which uses PARI up to reduction of the pairing,
coincides with the old Sage implementation (still used e.g. in
SageMathCell, but not available in the current code anymore)
To help with the database update, we adapt these tests to not be
dependent on the exact minimal polynomial used for finite field
generation:
- We turn both tests into examples (with fixed moduli) for their
respective methods
- We modify the test of `.weil_pairing` to check for two randomly chosen
`7282`-torsion points whether `algorithm="sage"` and `algorithm="pari"`
yield the same result
Please let me know if you think other polynomial-independent tests are
better suited as replacements.
See sagemath/conway-polynomials#5 (especially [comment
10](https://github.com/sagemath/conway-
polynomials/pull/5#issuecomment-2101462886) and Update 2 in [comment
11](https://github.com/sagemath/conway-
polynomials/pull/5#issuecomment-2101498241)) for more details.
CC: @yyyyx4 @GiacomoPope @JohnCremona
URL: sagemath#37967
Reported by: Sebastian A. Spindler
Reviewer(s):
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
May 12, 2024
…or Conway database update
As part of the Conway polynomials database update, two tests in
`schemes.elliptic_curves.ell_point` caused issues due to their
dependency on the precise implementation of the finite field
`GF(65537^2)`:
- The first test (of `.weil_pairing`) checks whether the Sage
implementation still works for an example of fixed `7282`-torsion points
- The second test (of `.tate_pairing`) checks whether the new
implementation, which uses PARI up to reduction of the pairing,
coincides with the old Sage implementation (still used e.g. in
SageMathCell, but not available in the current code anymore)
To help with the database update, we adapt these tests to not be
dependent on the exact minimal polynomial used for finite field
generation:
- We turn both tests into examples (with fixed moduli) for their
respective methods
- We modify the test of `.weil_pairing` to check for two randomly chosen
`7282`-torsion points whether `algorithm="sage"` and `algorithm="pari"`
yield the same result
Please let me know if you think other polynomial-independent tests are
better suited as replacements.
See sagemath/conway-polynomials#5 (especially [comment
10](https://github.com/sagemath/conway-
polynomials/pull/5#issuecomment-2101462886) and Update 2 in [comment
11](https://github.com/sagemath/conway-
polynomials/pull/5#issuecomment-2101498241)) for more details.
CC: @yyyyx4 @GiacomoPope @JohnCremona
URL: sagemath#37967
Reported by: Sebastian A. Spindler
Reviewer(s):
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.
As part of the Conway polynomials database update, two tests in
schemes.elliptic_curves.ell_pointcaused issues due to their dependency on the precise implementation of the finite fieldGF(65537^2):.weil_pairing) checks whether the Sage implementation still works for an example of fixed7282-torsion points.tate_pairing) checks whether the new implementation, which uses PARI up to reduction of the pairing, coincides with the old Sage implementation (still used e.g. in SageMathCell, but not available in the current code anymore)To help with the database update, we adapt these tests to not be dependent on the exact minimal polynomial used for finite field generation:
.weil_pairingto check for two randomly chosen7282-torsion points whetheralgorithm="sage"andalgorithm="pari"yield the same resultPlease let me know if you think other polynomial-independent tests are better suited as replacements.
See sagemath/conway-polynomials#5 (especially comment 10 and Update 2 in comment 11) for more details.
CC: @yyyyx4 @GiacomoPope @JohnCremona