Opened 13 years ago

Closed 7 years ago

#6881 closed enhancement (fixed)

Solving conics over polynomial rings.

Reported by: Victor S. Miller Owned by: Lennart Ackermans
Priority: major Milestone: sage-6.10
Component: algebraic geometry Keywords: conic, curve, function field
Cc: Marco Streng Merged in:
Authors: Lennart Ackermans Reviewers: Marco Streng
Report Upstream: N/A Work issues:
Branch: 995229c (Commits, GitHub, GitLab) Commit: 995229c89fd78d2f4f0782b1983537b4790a8222
Dependencies: Stopgaps:

Status badges

Description (last modified by Marco Streng)

Implement the algorithm of John Cremona and Mark van Hoeij for solving conics over fraction fields of polynomial rings.

The article http://www.warwick.ac.uk/~masgaj/papers/conicFT.pdf contains references to implementations in Maple (van Hoeij) and Magma (Cremona-Roberts), but these implementations heavily rely on things specific to these systems.

#727 provides classes for conic curves that this code should build upon

Change History (36)

comment:1 Changed 13 years ago by Alex Ghitza

Component: algebraalgebraic geometry

comment:2 Changed 12 years ago by Marco Streng

Report Upstream: N/A

See #727 A patch defining a conic class and using Simon's algorithms for finding points over Q is in progress.

comment:3 Changed 12 years ago by Marco Streng

Cc: Marco Streng added

comment:4 Changed 12 years ago by Marco Streng

Description: modified (diff)
Summary: ConicsSolving conics over polynomial rings.

I changed the description to better fit what is already in #727. Besides things that are already in #727, all that I removed from the original description were the following two requests.

  1. Use John Cremona's algorithms for finding points on conics over QQ.

It seems that Simon's algorithms (in #727) are better, but that doesn't have to stop us from giving Cremona's code as an option. It is inside mwrank, which is part of Sage. If someone wants to do it, then it can be made into a separate ticket.

  1. Getting primes of bad reduction of conics.

This is as good as in #727: make a Conic C. Then do C.determinant().factor()

comment:5 Changed 7 years ago by Lennart Ackermans

Owner: changed from tbd to Lennart Ackermans

comment:6 Changed 7 years ago by Lennart Ackermans

Branch: public/conics_rational_function_field

comment:7 Changed 7 years ago by git

Commit: 10cfe65b0ea45af8603d3a48aede6c2b6aa55cda

Branch pushed to git repo; I updated commit sha1. New commits:

10cfe65Continuing has_rational_point() on conics over rational function fields. Currently returns incorrect solution.

comment:8 Changed 7 years ago by git

Commit: 10cfe65b0ea45af8603d3a48aede6c2b6aa55cda73b7b05dfd2584ccb29964310d3241aee1d1f82b

Branch pushed to git repo; I updated commit sha1. New commits:

73b7b05Fixed has_rational_point and added some documentation

comment:9 Changed 7 years ago by git

Commit: 73b7b05dfd2584ccb29964310d3241aee1d1f82b8f4d0f46e9a4b021e0d113bf80c636f2be038d2a

Branch pushed to git repo; I updated commit sha1. New commits:

8f4d0f4Added documentation and bugfixes to con_rational_function_field

comment:10 Changed 7 years ago by Lennart Ackermans

Authors: Victor MillerLennart Ackermans

comment:11 Changed 7 years ago by Lennart Ackermans

Status: newneeds_review

comment:12 Changed 7 years ago by git

Commit: 8f4d0f46e9a4b021e0d113bf80c636f2be038d2a8a9d34fd202d381912a41e7b189fad21b0126408

Branch pushed to git repo; I updated commit sha1. New commits:

8a9d34fFixed documentation bug

comment:13 Changed 7 years ago by Marco Streng

Description: modified (diff)
Keywords: function field added
Reviewers: Marco Streng

comment:14 Changed 7 years ago by Marco Streng

Description: modified (diff)

comment:15 Changed 7 years ago by Marco Streng

Milestone: sage-featuresage-6.10
Priority: minormajor

comment:16 Changed 7 years ago by Marco Streng

Status: needs_reviewneeds_work

Not 100% doctest coverage, and there is a doctest that fails.

comment:17 Changed 7 years ago by git

Commit: 8a9d34fd202d381912a41e7b189fad21b0126408c6d8368fa1c7ccb579c9f430da111f4fd09c2069

Branch pushed to git repo; I updated commit sha1. New commits:

37d8b92Documentation update
c6d8368Merge branch 'public/conics_rational_function_field' of git://trac.sagemath.org/sage into con_rational_function_field

comment:18 in reply to:  16 Changed 7 years ago by Lennart Ackermans

Status: needs_workneeds_review

Replying to mstreng:

Not 100% doctest coverage, and there is a doctest that fails.

Should be okay now.

comment:19 Changed 7 years ago by git

Commit: c6d8368fa1c7ccb579c9f430da111f4fd09c2069b153ba50e28d8b7f744ee469f22af0027888f496

Branch pushed to git repo; I updated commit sha1. New commits:

b153ba5Improvements and bug fixes

comment:20 Changed 7 years ago by Marco Streng

Status: needs_reviewneeds_work

comment:21 Changed 7 years ago by git

Commit: b153ba50e28d8b7f744ee469f22af0027888f49649ecc851a3fc2c3f60ae84984426ac61bf8c6db4

Branch pushed to git repo; I updated commit sha1. New commits:

49ecc85Some small adjustments

comment:22 Changed 7 years ago by Lennart Ackermans

Status: needs_workneeds_review

comment:23 Changed 7 years ago by git

Commit: 49ecc851a3fc2c3f60ae84984426ac61bf8c6db4ee31fca60f6045b675dde0186f84f8eb983d8bf6

Branch pushed to git repo; I updated commit sha1. New commits:

ee31fcaSome small adjustments (2)

comment:24 Changed 7 years ago by Marco Streng

Status: needs_reviewpositive_review

All tests pass. Documentation looks good.

The functions do not work perfectly in all cases due to #20003, but after bypassing squarefree_decomposition, I get:

sage: K.<t> = PolynomialRing(GF(7))
sage: C = Conic([5*t^2+4, t^2+3*t+3, 6*t^2+3*t+2, 5*t^2+5, 4*t+3, 4*t^2+t+5])
sage: C.has_rational_point()
True

and

sage: F = FiniteField(7)
sage: P.<t> = F[]                                                               
sage: K = P.fraction_field()
sage: for i in range(50):                                                       
    c = [K.random_element() for j in range(6)]
    C = Conic(c)
    C.has_rational_point(point=True)
....:     
(False, None)
(False, None)
(False, None)
(False, None)
(True,
 ((2*t^8 + 5*t^7 + 6*t^6 + 5*t^5 + 4*t^4 + 5*t^2 + 3*t + 2)/(t^8 + 5*t^7 + 5*t^6
 + 4*t^5 + 3*t^4 + 2*t^3 + t^2 + 5*t + 6) : (t^8 + 2*t^7 + t^5 + t^4 + 2*t^3 + 2
*t^2 + 4*t + 4)/(t^8 + 6*t^7 + t^6 + 4*t^5 + t^4 + 2*t^2 + 5*t + 3) : 1))
(False, None)
(False, None)
(False, None)
(True,
 ((2*t^8 + t^7 + 6*t^6 + 4*t^5 + 6*t^4 + 5*t^2 + 1)/(t^8 + 2*t^7 + 6*t^6 + 3*t^5
 + 3*t^4 + 6*t^3 + 6*t^2 + 6*t) : (2*t^8 + 4*t^7 + 5*t^6 + 3*t^5 + t^4 + 5*t + 1
)/(t^8 + 2*t^7 + 6*t^6 + 3*t^5 + 3*t^4 + 6*t^3 + 6*t^2 + 6*t) : 1))
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(True,
 ((2*t^4 + 2*t^3 + 3*t^2)/(t^7 + t^6 + 5*t^4 + t^2 + t + 4) : (5*t^7 + 3*t^6 + t
^5 + 5*t^4 + 6*t^3 + 2*t^2 + 4*t)/(t^7 + t^6 + 5*t^4 + t^2 + t + 4) : 1))
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(True,
 ((t^8 + 5*t^7 + 2*t^6 + 2*t^5 + 3*t^3 + t^2 + 3)/(t^7 + t^6 + 6*t^4 + 5*t^3 + 6
*t^2 + 2*t + 2) : (4*t^5 + 2*t^4 + 5*t^3 + 4*t^2 + 5*t + 2)/(t^4 + 2*t^3 + 5*t^2
 + 4*t + 5) : 1))
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(False, None)
(True,
 ((2*t^11 + 3*t^10 + 6*t^9 + t^8 + 6*t^7 + 4*t^6 + t^5 + 4*t^4 + 2*t^3 + 3*t + 5
)/(t^11 + 5*t^10 + 5*t^9 + t^8 + t^7 + 6*t^6 + 6*t^5 + 4*t^4 + 6*t^3 + 4*t) : (2
*t^9 + 4*t^8 + 4*t^7 + 6*t^6 + 5*t^5 + 5*t^4 + 4*t^3 + t^2 + 6*t + 1)/(t^10 + 5*
t^8 + 4*t^7 + 2*t^6 + 3*t^5 + 5*t^4 + 6*t^2 + 5*t) : 1))
(False, None)
(False, None)
(False, None)
(False, None)

comment:25 Changed 7 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

Please rebase to sage-7.1.beta2 (in particular, sage.rings.arith has moved to sage.arith) and use the standard copyright template for the newly added file.

If you have done this, you can set this ticket back to positive review.

Version 0, edited 7 years ago by Jeroen Demeyer (next)

comment:26 Changed 7 years ago by Jeroen Demeyer

You might also want to fix these pyflakes warnings:

src/sage/schemes/plane_conics/con_rational_function_field.py:25: 'NumberField' imported but unused
src/sage/schemes/plane_conics/con_rational_function_field.py:26: 'identity_matrix' imported but unused
src/sage/schemes/plane_conics/con_rational_function_field.py:32: redefinition of unused 'vector' from line 30

comment:27 Changed 7 years ago by git

Commit: ee31fca60f6045b675dde0186f84f8eb983d8bf696c2cc84c16865f36d9833d6745ab8f4c9a995fc

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

333fe2fFixed has_rational_point and added some documentation
038caecAdded documentation and bugfixes to con_rational_function_field
7d0e94aDocumentation update
de731feFixed documentation bug
fbd8543Improvements and bug fixes
81d6407Some small adjustments
731d696Some small adjustments (2)
483349eSome small adjustments (3)
004758eFixed conflict
96c2cc8Rebase to 7.1.beta2

comment:28 Changed 7 years ago by Lennart Ackermans

Status: needs_workpositive_review

comment:29 Changed 7 years ago by Jeroen Demeyer

Status: positive_reviewneeds_review

10 new commits? Sorry, but this needs to be reviewed (not by me).

comment:30 Changed 7 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

This is malformatted:

EXAMPLES:
    
    Create a conic::

        sage: K = FractionField(PolynomialRing(QQ, 't'))

It should be

EXAMPLES:
    
Create a conic::

    sage: K = FractionField(PolynomialRing(QQ, 't'))

comment:31 in reply to:  25 ; Changed 7 years ago by Jeroen Demeyer

Please do this:

Replying to jdemeyer:

use the standard copyright template for the newly added file.

comment:32 in reply to:  29 Changed 7 years ago by Lennart Ackermans

Replying to jdemeyer:

10 new commits? Sorry, but this needs to be reviewed (not by me).

Only the last 3 are new

comment:33 Changed 7 years ago by git

Commit: 96c2cc84c16865f36d9833d6745ab8f4c9a995fc995229c89fd78d2f4f0782b1983537b4790a8222

Branch pushed to git repo; I updated commit sha1. New commits:

995229cStandard copyright template

comment:34 in reply to:  31 Changed 7 years ago by Lennart Ackermans

Status: needs_workneeds_review

Replying to jdemeyer:

Please do this:

Replying to jdemeyer:

use the standard copyright template for the newly added file.

Sorry, I misunderstood your last comment. All should be okay now.

comment:35 Changed 7 years ago by Marco Streng

Status: needs_reviewpositive_review

Thanks Jeroen. I checked the changes and documentation html, and I ran the doctests again.

comment:36 Changed 7 years ago by Volker Braun

Branch: public/conics_rational_function_field995229c89fd78d2f4f0782b1983537b4790a8222
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.