New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add order setters and Tate and ate pairings #10912
Comments
Attachment: 10912.patch.gz |
comment:1
Excellent work. I'll need some time to go through the Tate and Ate pairing code; if no-one gets to it first this is something to do at SD29 in 10 days' time. Until then, just two comments:
|
comment:2
Response to the two comments:
|
comment:3
When I tried timing the first example in the tate_pairing, I got the following error:
Whereas without the timing everthing is fine:
|
comment:4
Ok, I can be more specific in what the error is. In tate_pairing (P.tate_pairing(Q,n,k)), if P.miller(Q, n) raises a
|
Attachment: Trac10912.patch.gz This fixed the bug in tate_pairing. |
comment:5
I posted a patch which replaces the original. It fixes the bug found in tate_pairing. So far it doesn't do anything to make the group order setting more robust. |
comment:6
It also now uses Hasse_bounds in the group set_order. |
comment:7
So is this ready for review then? |
Changed author from Mariah Lenox to Mariah Lenox, Aly Deines |
This comment has been minimized.
This comment has been minimized.
Attachment: trac_10912-ate.patch.gz Reviewer patch: applies to 4.7.alpha5 |
comment:10
Looks fine to me, and all tests in elliptic)curves pass. Very nice job with excellent tests and examples. My patch replaces the previous ones; it just corrected a couple of minor typos in the docstrings. Positive review! |
This comment has been minimized.
This comment has been minimized.
Reviewer: John Cremona |
Work Issues: documentation formatting |
comment:12
There are some issues with the formatting of the documentation:
The INPUT and OUTPUT blocks should not be indented, see http://sagemath.org/doc/developer/conventions.html#docstring-markup-with-rest-and-sphinx |
Attachment: trac_10912-ate.p2.patch.gz diff for review purposes only |
This comment has been minimized.
This comment has been minimized.
comment:13
Attachment: trac_10912-ate.p2.diff.gz Attachment attachment: trac_10912-ate.p2.patch makes the INPUT and OUTPUT blocks not be indented. Nothing else was changed from attachment [attachment: trac_10912-ate.patch]. |
comment:14
Something is wrong, the new patch is much smaller than the old patch. |
Attachment: trac_10912-ate.p3.patch.gz |
diff for review purposes only |
comment:15
Attachment: trac_10912-ate.p3.diff.gz Oops! Forgot a file. Apologies. Attachment [attachment: trac_10912-ate.p3.diff] is a diff of |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
comment:16
Attachment: 10912_doc_reviewer.patch.gz Positive review to the documentation changes. Somebody still needs to review my reviewer patch. |
Changed reviewer from John Cremona to John Cremona, Jeroen Demeyer |
Changed work issues from documentation formatting to none |
comment:18
bump Anyone wants to review my patch? |
Merged: sage-4.7.1.alpha2 |
add order setters and Tate and ate pairings
Apply:
Component: elliptic curves
Author: Mariah Lenox, Aly Deines
Reviewer: John Cremona, Jeroen Demeyer
Merged: sage-4.7.1.alpha2
Issue created by migration from https://trac.sagemath.org/ticket/10912
The text was updated successfully, but these errors were encountered: