Opened 15 years ago
Closed 12 years ago
#727 closed enhancement (fixed)
find rational points on plane conic curves
Reported by: | William Stein | Owned by: | Marco Streng |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6.2 |
Component: | geometry | Keywords: | rational point points conic quadratic form |
Cc: | ncalexan, Justin Walker, Marco Streng, Robert Miller, Peter Bruin | Merged in: | sage-4.6.2.alpha2 |
Authors: | Nick Alexander, Marco Streng | Reviewers: | David Loeffler |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Implement a Conic class that is able to
- find rational points on plane conic curves over QQ, finite fields, RR, CC.
- given a conic with a point over a field, compute a parametrization.
Attachments (13)
Change History (34)
comment:1 Changed 15 years ago by
Component: | basic arithmetic → algebraic geometry |
---|---|
Owner: | changed from somebody to William Stein |
comment:2 Changed 15 years ago by
Cc: | ncalexan added |
---|---|
Keywords: | rational point points conic quadratic form added |
comment:3 Changed 14 years ago by
Component: | algebraic geometry → quadratic forms |
---|---|
Owner: | changed from William Stein to Justin Walker |
comment:4 Changed 14 years ago by
Cc: | Justin Walker added |
---|
comment:5 Changed 13 years ago by
Cc: | Marco Streng added |
---|
Changed 13 years ago by
Attachment: | trac_727-conic.patch added |
---|
Define a Conic class with an interface to Denis Simon's qfsolve
comment:6 Changed 13 years ago by
Component: | quadratic forms → geometry |
---|---|
Owner: | changed from Justin Walker to mhampton |
Summary: | find rational points on ternary quadratic forms -- volunteer needed → find rational points on plane conic curves [with patch, needs work] |
The patches I just uploaded were part of ncalexan's patch for ticket 6341, implementing Mestre's algorithm. They implement a Conic class and allow one to find points over the rationals. This Conic class needs to be expanded to provide much more functionality, such as finding points and/or rational parametrizations over other fields. If the original question is only for the rationals, then that at least is answered.
comment:7 Changed 13 years ago by
Owner: | changed from mhampton to Marco Streng |
---|---|
Status: | new → assigned |
I'll work on the Conic class.
Changed 12 years ago by
Attachment: | trac_727_more_conic_files.patch added |
---|
comment:8 Changed 12 years ago by
Cc: | Robert Miller added |
---|---|
Report Upstream: | → N/A |
- this is being revived at Sage Days 23
- trac_727-conic-extcode.patch should be removed: it updates Denis Simon's code from 2005 to 2007, while 2009 is already shipped with the latest Sage
- trac_727_more_conic_files.patch implements a Conic class that uses Denis Simon's code and can find points on conics over number fields and finite fields. It includes most of trac_727-conic.patch and works by itself on 4.4.4. Some doctests fail. It includes an incomplete and incorrect implementation of Hilbert symbols that should be replaced by the Hilbert symbols of #9334
comment:9 Changed 12 years ago by
- I missed the file sage/schemes/plane_conics/init.py (which is empty), so the patch in its current form doesn't work
Changed 12 years ago by
Attachment: | trac_727_more_conic_files2.patch added |
---|
apply this patch and the one without "2" to get something that almost works
Changed 12 years ago by
Attachment: | trac_727_more_conic_files3.patch added |
---|
needs two previous tickets trac_727_more... and trac_9334-hilbert.patch
Changed 12 years ago by
Attachment: | trac_727_more_conic_files4.patch added |
---|
apply after trac_727_more_conic_files1,2,3; requires trac_9334-hilbert.patch to work
Changed 12 years ago by
Attachment: | trac_727_more_conic_files5.patch added |
---|
apply on sage 4.4.4 after trac_727_more_conic_files1,2,3,4, shouldn't require trac_9334-hilbert.patch
Changed 12 years ago by
Attachment: | trac_727_more_conic_files6.patch added |
---|
apply on sage 4.4.4 after trac_727_more_conic_files1,2,3,4,5
Changed 12 years ago by
Attachment: | trac_727_conic_combined_1-6.patch added |
---|
apply only this latest file
comment:10 Changed 12 years ago by
Authors: | → Nick Alexander, Marco Streng |
---|---|
Cc: | Peter Bruin added |
Description: | modified (diff) |
Work issues: | → documentation formatting, reviewing |
Changed 12 years ago by
Attachment: | trac_727_more_conic_files7.patch added |
---|
apply the 1-6 patch on sage 4.5.1, then apply this one
Changed 12 years ago by
Attachment: | trac_727_more_conic_files9.patch added |
---|
apply after 7 (there is no 8)
comment:11 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Work issues: | documentation formatting, reviewing |
I'm running a doctest right now. I'll fold them when I get the time. Who wants to review it?
Changed 12 years ago by
Attachment: | trac_727_conic_combined_1-9.patch added |
---|
apply only this latest file (works on sage 4.4.4 and on 4.5.1)
comment:12 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
"All tests passed!", and I combined the patches into one file
comment:13 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:14 Changed 12 years ago by
Summary: | find rational points on plane conic curves [with patch, needs work] → find rational points on plane conic curves [with patch, needs review] |
---|
comment:15 Changed 12 years ago by
Summary: | find rational points on plane conic curves [with patch, needs review] → find rational points on plane conic curves |
---|
Those [with patch, needs review]
bits in the description are no longer necessary, thanks to the workflow.
comment:16 Changed 12 years ago by
Status: | needs_review → needs_work |
---|
The patch applies fine to 4.6.alpha0, but not all the tests in the new directory pass:
sage -t -long "devel/sage-main/sage/schemes/plane_conics/con_field.py" sage -t -long "devel/sage-main/sage/schemes/plane_conics/con_number_field.py" sage -t -long "devel/sage-main/sage/schemes/plane_conics/con_rational_field.py" sage -t -long "devel/sage-main/sage/schemes/plane_conics/rnfisnorm.py"
I feared that it would be worse after the new Pari upgrade (which is in 4.6.alpha0). But it does need looking into.
comment:17 Changed 12 years ago by
Changed 12 years ago by
Attachment: | trac_727_conics.patch added |
---|
comment:18 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Work issues: | → needs serious retesting after #9334 and #2329 |
Changed 12 years ago by
Attachment: | trac_727_conics_without_number_fields.patch added |
---|
Apply only this latest file (works on sage 4.6.1.rc0 without any other patches)
comment:19 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_review |
Work issues: | needs serious retesting after #9334 and #2329 |
I moved all number field functionality to #10621. Because of this, no patches from other tickets are needed, you only need to apply trac_727_conics_without_number_fields.patch
comment:20 Changed 12 years ago by
Reviewers: | → David Loeffler |
---|---|
Status: | needs_review → positive_review |
This looks really good. It's embarrassing that we've been shipping Denis Simon's scripts with Sage for years but nobody's sat down and written the interface code necessary to make it accessible.
Patch applied fine to 4.6.2.alpha1, all doctests in sage/schemes passed, the ref manual built OK, and all the examples I tried worked.
I have one minor gripe: there are one or two cases where the new Conic classes inherit methods from the generic plane curve classes that perhaps ought to be replaced with more appropriate conic-specific implementations (e.g the method "rational_points"). But that can come in future tickets if people feel it's needed.
comment:21 Changed 12 years ago by
Merged in: | → sage-4.6.2.alpha2 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
This is so random -- I have written code to do this. We already distribute Denis Simon's code to solve such systems, and I have have integrated a little of it. Patch coming up. Please talk to me before implementing this.