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

Integral points on elliptic curves over number fields #10973

Open
RalphieBoy mannequin opened this issue Mar 21, 2011 · 108 comments
Open

Integral points on elliptic curves over number fields #10973

RalphieBoy mannequin opened this issue Mar 21, 2011 · 108 comments

Comments

@RalphieBoy
Copy link
Mannequin

RalphieBoy mannequin commented Mar 21, 2011

Incorporate work done by Rado Kirov and Jackie Anderson at Sage Days 22, based on Magma implementation by Cremona's student Nook.

See also #12095 (now tagged as duplicate).

Converted to git by John Cremona, 2013-12-18

CC: @JohnCremona @sagetrac-gagansekhon @jbalakrishnan

Component: elliptic curves

Keywords: sd32

Author: Justin Walker, Aly Deines, Jennifer Balakrishnan

Branch/Commit: public/10973 @ 0774d59

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

@RalphieBoy
Copy link
Mannequin Author

RalphieBoy mannequin commented Mar 21, 2011

Attachment: intpts.sage.gz

@RalphieBoy RalphieBoy mannequin added the t: enhancement label Mar 21, 2011
@RalphieBoy
Copy link
Mannequin Author

RalphieBoy mannequin commented Mar 22, 2011

Attachment: ell_int_points.py.gz

This code goes into "sage/schemes/elliptiic_curves". It works, but needs work.

@RalphieBoy
Copy link
Mannequin Author

RalphieBoy mannequin commented Mar 22, 2011

comment:2

New version of .py file, fixing a problem with bogus interaction with Maxima.

@sagetrac-gagansekhon
Copy link
Mannequin

sagetrac-gagansekhon mannequin commented Mar 23, 2011

Attachment: trac_10973.gz

@RalphieBoy
Copy link
Mannequin Author

RalphieBoy mannequin commented Mar 23, 2011

comment:3

The patch trac_10973 should apply w/o problems against sage-4.7.alpha1.

The previous attachments are replaced by this one.

@RalphieBoy
Copy link
Mannequin Author

RalphieBoy mannequin commented Mar 25, 2011

comment:4

This patch is related to, and appears to fix, the problem described by #10152.

@RalphieBoy
Copy link
Mannequin Author

RalphieBoy mannequin commented Mar 25, 2011

Attachment: trac_10973.2.gz

Updated patch with code to reject requests w/o generators; replaces previous patch of same name.

@sagetrac-gagansekhon
Copy link
Mannequin

sagetrac-gagansekhon mannequin commented Mar 25, 2011

comment:5

minor error in all.py, +from ell_int_points.py import *
should be +from ell_int_points import *
The new patch that I will be uploading in a minute fixes that.

@sagetrac-gagansekhon
Copy link
Mannequin

sagetrac-gagansekhon mannequin commented Mar 25, 2011

Attachment: trac_10973.patch.gz

@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

Attachment: trac_10973.2.patch.gz

This replaces the previous patch

@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

comment:7

The tests pass on all files except ell_ergos.py and constructor.py, specifically, the tests which call S_integral_points([]) are having issues.

All examples from the paper referenced now pass as do all examples from #10152, so once this is implemented, this also fixes #10152.

@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

replaces previous patch

@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

comment:8

Attachment: trac_10973.3.patch.gz

This should do it. This is a small fix to the previous patch, it now passes the tests.

@adeines adeines mannequin added the s: needs review label Aug 24, 2011
@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

Changed author from Justin Walker to Justin Walker, Aly Deines

@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

Changed author from Justin Walker, Aly Deines to Justin Walker, Aly Deines, Jennifer Balakrishnan

@williamstein
Copy link
Contributor

Attachment: report.txt

This is a list of 14 issues I had reading through trac_10973.3.patch.

@williamstein
Copy link
Contributor

comment:11

Please see the rather long file report.txt that I posted with a bunch of issues I noticed when reading through trac_10973.3.patch.

@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

Attachment: Trac10973.4.patch.gz

Replaces previous patch.

@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

comment:12

Trac10973.4.patch addresses everything in report.txt.

This returns the same result as the original integral_points over Q with all curves of conductor up to 1000 (and also agrees with Magma). This also finds the missing points referred to in ticket #10152.

We compared times on all curves over Q of conductor less then 1000; the original implementation was usually 2 to 8
times faster, though in certain instances it did not find all integral points (this was ticket #10152). So this is a bit of a slowdown.

If E is a curve over Q, then E.integral_points() calls the method in ell_rational_points.py (the previous, faster, not always correct, method).

@adeines adeines mannequin added s: needs review and removed s: needs work labels Aug 24, 2011
@adeines
Copy link
Mannequin

adeines mannequin commented Aug 24, 2011

Attachment: Trac10973.5.patch.gz

Replaces previous patch, corrects Authors.

@williamstein
Copy link
Contributor

comment:13

Replying to @adeines:

If E is a curve over Q, then E.integral_points() calls the
method in ell_rational_points.py (the previous, faster, not always correct, method).

Why? Surely it would definitely be better to return a correct answer by default. I can imagine leaving the old code in with an not-by-default option to call it and docs that mention it is faster.

@JohnCremona
Copy link
Member

comment:73

I have a current branch called intptsQ which I made good progress on in December and will get back to.

@embray
Copy link
Contributor

embray commented Mar 25, 2019

comment:74

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

@embray embray modified the milestones: sage-8.7, sage-8.8 Mar 25, 2019
@embray
Copy link
Contributor

embray commented Jun 14, 2019

comment:75

Tickets still needing working or clarification should be moved to the next release milestone at the soonest (please feel free to revert if you think the ticket is close to being resolved).

@embray embray modified the milestones: sage-8.8, sage-8.9 Jun 14, 2019
@embray
Copy link
Contributor

embray commented Dec 30, 2019

comment:76

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-8.9, sage-9.1 Dec 30, 2019
@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 14, 2020

comment:77

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 Apr 14, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Sep 5, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 13, 2021

comment:79

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 19, 2021

comment:80

Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 2, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
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

7 participants