Skip to content
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 rational_points over plane curves AND addition of rational_points_iterator function #8428

Closed
sagetrac-cturner mannequin opened this issue Mar 3, 2010 · 11 comments

Comments

@sagetrac-cturner
Copy link
Mannequin

sagetrac-cturner mannequin commented Mar 3, 2010

The newly "improved" rational_points function for projective plane curves (#8193) has a bug; if for some (Y,Z) the polynomial defining the curve becomes identically 0, it returns a ValueError caused by the function trying to factorise 0 as a polynomial.

Here is an example

sage: F = GF(2)
sage: P2.<X,Y,Z> = ProjectiveSpace(F,2)
sage: C = Curve(X*Y)
sage: a = C.rational_points_iterator()
sage: a.next()
(1 : 0 : 0)
sage: a.next()
(0 : 1 : 0)
sage: a.next()
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)

/home/charlie/<ipython console> in <module>()

/home/charlie/sage-current/local/lib/python2.6/site-packages/sage/schemes/plane_curves/projective_curve.pyc
in rational_points_iterator(self)
   353         # points with Z = 1
   354         for y in K:
--> 355             for x in R(g(X,y,one)).roots(multiplicities=False):
   356                 yield(self.point([x,y,one]))
   357

/home/charlie/sage-current/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so
in sage.rings.polynomial.polynomial_element.Polynomial.roots
(sage/rings/polynomial/polynomial_element.c:30111)()

/home/charlie/sage-current/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element.so
in sage.rings.polynomial.polynomial_element.Polynomial.factor
(sage/rings/polynomial/polynomial_element.c:18463)()
ValueError: factorization of 0 not defined

I propose to write new rational_points_iterator function that will return an iterator over the set of rational points of the projective plane curve. It will avoid this bug. It will be called by rational_points to return a list of all these rational points. A patch to do all of this is on its way.

CC: @JohnCremona

Component: algebraic geometry

Author: Charlie Turner

Reviewer: David Roe

Merged: sage-4.4.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/8428

@sagetrac-cturner

This comment has been minimized.

@sagetrac-cturner

This comment has been minimized.

@sagetrac-cturner sagetrac-cturner mannequin changed the title Problem with rational_points over plane curves Problem with rational_points over plane curves AND addition of rational_points_iterator function Mar 3, 2010
@sagetrac-cturner

This comment has been minimized.

@sagetrac-cturner
Copy link
Mannequin Author

sagetrac-cturner mannequin commented Mar 4, 2010

Applies to 4.3.3

@sagetrac-cturner
Copy link
Mannequin Author

sagetrac-cturner mannequin commented Mar 4, 2010

Attachment: trac_8428_rational_points_iterator.2.patch.gz

Applies to 4.3.3

@sagetrac-cturner
Copy link
Mannequin Author

sagetrac-cturner mannequin commented Mar 4, 2010

comment:5

Attachment: trac_8428_rational_points_iterator.patch.gz

@sagetrac-cturner
Copy link
Mannequin Author

sagetrac-cturner mannequin commented Mar 4, 2010

comment:6

Apologies; both copies of the patch are the same, I double-clicked and am unable to remove the 2nd copy.

@sagetrac-cturner sagetrac-cturner mannequin assigned sagetrac-cturner and aghitza and unassigned aghitza and sagetrac-cturner Mar 4, 2010
@roed314
Copy link
Contributor

roed314 commented Mar 13, 2010

comment:8

Fixes the bug, doctested and doctests pass. Positive review.

@roed314
Copy link
Contributor

roed314 commented Mar 13, 2010

Reviewer: David Roe

@roed314 roed314 added this to the sage-4.3.4 milestone Mar 13, 2010
@jhpalmieri
Copy link
Member

Merged: sage-4.4.alpha0

@jhpalmieri
Copy link
Member

comment:9

Merged "trac_8428_rational_points_iterator.patch" into 4.4.alpha0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants