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
Problem with points at infinity in hyperelliptic curves #11800
Comments
comment:1
The Author(s) field refers to authors of attached code, e.g. patches fixing a bug, not (necessarily the same as) those who opened a ticket. In short, if there's no code, the Author(s) field should be empty. |
Changed author from Pierrick Gaudry, Paul Zimmermann to none |
Changed keywords from none to ecc2011 |
The patch bans defining polynomials of degree two for hyperelliptic curves, and corrects two typos. |
comment:5
Attachment: trac_11800_ban_degree_two.patch.gz The problem arises exactly when As far as I can tell, it is not checked at the moment whether the curve defined by |
Author: David Eklund |
Work Issues: rebase |
Dependencies: #11930 |
Attachment: 11800.patch.gz rebase to apply after #11930 |
comment:8
apply attachment: 11800.patch |
This comment has been minimized.
This comment has been minimized.
Changed work issues from rebase to none |
Changed keywords from ecc2011 to ecc2011, sd35, hyperelliptic curve, conic |
comment:10
Actually, I don't see any reason to ban conics from being hyperelliptic curves in Sage. Mathematically, I would say that a hyperelliptic curve has genus >= 2, but I don't want to forbid people to use the HyperelllipticCurve classes for lower-genus curves. For example, a user may have some loop going on where curves sometimes have genus 0, but sometimes higher. The point Rather than disallowing conics, we could make a separate case in the |
comment:11
Ok thanks, I will watch out for consistency with #11930. I will start working on the points method so that it allows conics, that is the case where (0:1:0) does not lie on the curve. This appears to be the better option, since changing the points method seems perfectly safe and banning conics is controversial (at least to some degree). The problem when f is of degree 1 and h=0 (or more generally h=constant) stems from issues in the homogenization of |
comment:12
I changed the description to a simpler example (to include in the documentation) which illustrates the same issue. |
This comment has been minimized.
This comment has been minimized.
comment:14
Not related to the example in the ticket description or to the patch, but very much related to the title of this ticket and the first line of the description: what is "points" supposed to mean? I assume it means points over the base field, but of which model?
See #11980 |
comment:15
Points at infinity are counted incorrectly for degree 2 by this patch.
Here C and H represent the same curve. It is a smooth conic over a finite field of order 7, hence has 8 rational points, but only 6 are found. In general, if H has degree 2, there are 0 or 2 rational points at infinity (as in my previous comment). Since H has degree < 4, the plane model equals the smooth model, so these points can be represented and returned correctly in Sage. |
Reviewer: Marco Streng |
comment:16
In the present patch, the points() method returns all the rational points on the plane projective model, including those on the line at infinity. I agree that when the projective plane model is singular (the typical case) we really want the points() method to spit out points on a smooth model. Maybe the issue should be raised in a separate ticket. A comment on the matter by David Kohel: The approach of Magma is to use weighted projective space which allows one to represent two points at infinity (when the degree of |
comment:17
Replying to @sagetrac-davideklund:
Yes, this is a standard textbook approach to hyperelliptic curves. It is requested in the documentation of rational_points as a way of representing two points at infinity. I don't think weighted projective spaces exist in Sage, so a minimal implementation of weighted projective spaces (like Magma's Anyway, your patch looks good. I'll test it now. |
comment:18
I see that the new patch isn't set to "needs review" yet, but I'll comment on it anyway: I think some explanation of the situation at infinity is in order in the documentation. How about the following? "This method currently lists points in the plane projective model, which means that one point (0:1:0) at infinity is returned if the degree of the curve is at least 4. Later implementations may consider the more mathematically correct desingularisation at infinity, replacing (0:1:0) by 0 or 2 smooth rational points if the degree is even." + EXAMPLE of degree 6. Also, your method of finding points at infinity is linear in the field order, this can be improved by taking (1:y:0) and solving for y (i.e., extracting 1 square root), exactly as currently the affine points are found by taking (x:y:1) for each x and solving for y. |
comment:19
Replying to @mstreng:
Thank you for your comments! I hoped to attend to this earlier. I think I will get some time to work on it soon. Yes, some comment like the one you supply would be in order in the documentation. About the method for finding points: I used brute force since I deliberately choose simple code before efficiency. But I will implement the refined approach you suggest. |
Patch updated. More efficient method to find the points at infinity and added documentation. |
comment:20
Attachment: trac_11800_allow_conics.patch.gz |
comment:21
for patchbot: apply trac_11800_allow_conics.patch |
Attachment: 11800-reviewer.patch.gz break at 79 characters, shortened polynomial coefficient access |
Changed dependencies from #11930 to none |
This comment has been minimized.
This comment has been minimized.
comment:23
Python style guides ask for breaking of lines at 79 characters. Aside from that, the patch is good. If you agree with my reviewer patch, then feel free to set the ticket to "positive review". |
comment:24
Replying to @mstreng:
Ok, thanks! I agree with your changes. |
Merged: sage-5.0.beta11 |
comment:26
See #15115 for getting the correct points at infinity in general. |
The function that lists the points on a hyperelliptic curve assumes that the point [0, 1, 0] is always valid. This is wrong and creates the following bug:
Apply
CC: @sagetrac-nestibal @jpflori
Component: algebraic geometry
Keywords: ecc2011, sd35, hyperelliptic curve, conic
Author: David Eklund
Reviewer: Marco Streng
Merged: sage-5.0.beta11
Issue created by migration from https://trac.sagemath.org/ticket/11800
The text was updated successfully, but these errors were encountered: