Opened 13 years ago

Closed 7 years ago

# Solving conics over polynomial rings.

Reported by: Owned by: victor lackermans major sage-6.10 algebraic geometry conic, curve, function field mstreng Lennart Ackermans Marco Streng N/A 995229c 995229c89fd78d2f4f0782b1983537b4790a8222

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

### comment:1 Changed 13 years ago by AlexGhitza

• Component changed from algebra to algebraic geometry

### comment:2 Changed 12 years ago by mstreng

• Report Upstream set to N/A

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

### comment:4 Changed 12 years ago by mstreng

• Description modified (diff)
• Summary changed from Conics to Solving 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 lackermans

• Owner changed from tbd to lackermans

### comment:6 Changed 7 years ago by lackermans

• Branch set to public/conics_rational_function_field

### comment:7 Changed 7 years ago by git

• Commit set to 10cfe65b0ea45af8603d3a48aede6c2b6aa55cda

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

 ​10cfe65 `Continuing has_rational_point() on conics over rational function fields. Currently returns incorrect solution.`

### comment:8 Changed 7 years ago by git

• Commit changed from 10cfe65b0ea45af8603d3a48aede6c2b6aa55cda to 73b7b05dfd2584ccb29964310d3241aee1d1f82b

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

 ​73b7b05 `Fixed has_rational_point and added some documentation`

### comment:9 Changed 7 years ago by git

• Commit changed from 73b7b05dfd2584ccb29964310d3241aee1d1f82b to 8f4d0f46e9a4b021e0d113bf80c636f2be038d2a

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

 ​8f4d0f4 `Added documentation and bugfixes to con_rational_function_field`

### comment:10 Changed 7 years ago by lackermans

• Authors changed from Victor Miller to Lennart Ackermans

### comment:11 Changed 7 years ago by lackermans

• Status changed from new to needs_review

### comment:12 Changed 7 years ago by git

• Commit changed from 8f4d0f46e9a4b021e0d113bf80c636f2be038d2a to 8a9d34fd202d381912a41e7b189fad21b0126408

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

 ​8a9d34f `Fixed documentation bug`

### comment:13 Changed 7 years ago by mstreng

• Description modified (diff)
• Reviewers set to Marco Streng

### comment:14 Changed 7 years ago by mstreng

• Description modified (diff)

### comment:15 Changed 7 years ago by mstreng

• Milestone changed from sage-feature to sage-6.10
• Priority changed from minor to major

### comment:16 follow-up: ↓ 18 Changed 7 years ago by mstreng

• Status changed from needs_review to needs_work

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

### comment:17 Changed 7 years ago by git

• Commit changed from 8a9d34fd202d381912a41e7b189fad21b0126408 to c6d8368fa1c7ccb579c9f430da111f4fd09c2069

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

 ​37d8b92 `Documentation update` ​c6d8368 `Merge 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 lackermans

• Status changed from needs_work to needs_review

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

Should be okay now.

### comment:19 Changed 7 years ago by git

• Commit changed from c6d8368fa1c7ccb579c9f430da111f4fd09c2069 to b153ba50e28d8b7f744ee469f22af0027888f496

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

 ​b153ba5 `Improvements and bug fixes`

### comment:20 Changed 7 years ago by mstreng

• Status changed from needs_review to needs_work

### comment:21 Changed 7 years ago by git

• Commit changed from b153ba50e28d8b7f744ee469f22af0027888f496 to 49ecc851a3fc2c3f60ae84984426ac61bf8c6db4

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

 ​49ecc85 `Some small adjustments`

### comment:22 Changed 7 years ago by lackermans

• Status changed from needs_work to needs_review

### comment:23 Changed 7 years ago by git

• Commit changed from 49ecc851a3fc2c3f60ae84984426ac61bf8c6db4 to ee31fca60f6045b675dde0186f84f8eb983d8bf6

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

 ​ee31fca `Some small adjustments (2)`

### comment:24 Changed 7 years ago by mstreng

• Status changed from needs_review to positive_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 follow-up: ↓ 31 Changed 7 years ago by jdemeyer

• Status changed from positive_review to needs_work

Please rebase to sage-7.1.beta2 (in particular, `sage.rings.arith` has moved to `sage.arith.all`) 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.

Last edited 7 years ago by jdemeyer (previous) (diff)

### comment:26 Changed 7 years ago by jdemeyer

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 changed from ee31fca60f6045b675dde0186f84f8eb983d8bf6 to 96c2cc84c16865f36d9833d6745ab8f4c9a995fc

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

 ​333fe2f `Fixed has_rational_point and added some documentation` ​038caec `Added documentation and bugfixes to con_rational_function_field` ​7d0e94a `Documentation update` ​de731fe `Fixed documentation bug` ​fbd8543 `Improvements and bug fixes` ​81d6407 `Some small adjustments` ​731d696 `Some small adjustments (2)` ​483349e `Some small adjustments (3)` ​004758e `Fixed conflict` ​96c2cc8 `Rebase to 7.1.beta2`

### comment:28 Changed 7 years ago by lackermans

• Status changed from needs_work to positive_review

### comment:29 follow-up: ↓ 32 Changed 7 years ago by jdemeyer

• Status changed from positive_review to needs_review

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

### comment:30 Changed 7 years ago by jdemeyer

• Status changed from needs_review to needs_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:32 in reply to: ↑ 29 Changed 7 years ago by lackermans

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 changed from 96c2cc84c16865f36d9833d6745ab8f4c9a995fc to 995229c89fd78d2f4f0782b1983537b4790a8222

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

 ​995229c `Standard copyright template`

### comment:34 in reply to: ↑ 31 Changed 7 years ago by lackermans

• Status changed from needs_work to needs_review

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

### comment:35 Changed 7 years ago by mstreng

• Status changed from needs_review to positive_review

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

### comment:36 Changed 7 years ago by vbraun

• Branch changed from public/conics_rational_function_field to 995229c89fd78d2f4f0782b1983537b4790a8222
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.