Opened 7 years ago

Closed 7 years ago

#16009 closed enhancement (fixed)

Compute rank of elliptic curve defined over relative number field

Reported by: mmasdeu Owned by:
Priority: major Milestone: sage-6.2
Component: elliptic curves Keywords: descent, rank, elliptic curve, relative number field
Cc: cremona, pbruin Merged in:
Authors: Marc Masdeu Reviewers: Peter Bruin
Report Upstream: N/A Work issues:
Branch: 8bceb36 (Commits, GitHub, GitLab) Commit: 8bceb36e55fe51e86815f8432eec201d401595ea
Dependencies: #16022 Stopgaps:

Status badges

Description

Currently the following fails:

sage: x = QQ['x'].gen()
sage: F.<a> = QuadraticField(5)
sage: K.<b> = F.extension(x^2-3)
sage: E = EllipticCurve(K,[0,0,0,b,1])
sage: E.rank()

This is because the sage function wrapping around Denis Simon's two-descent script does not consider relative number fields.

I propose an easy fix for it.

Change History (14)

comment:1 Changed 7 years ago by mmasdeu

  • Branch set to u/mmasdeu/gp_simon_relative
  • Cc cremona pbruin added
  • Commit set to 2085e0c096a1a18a15d8dbcdde06627749b90542
  • Status changed from new to needs_review

New commits:

1dcdf8dAllow for relative number fields to be passed to two_descent function.
78bb894Fixed a doctest.
2085e0cAdded some comments.

comment:2 Changed 7 years ago by pbruin

  • Reviewers set to Peter Bruin
  • Status changed from needs_review to needs_work

Looks good and passes all tests. The new doctest does take a long time (4 minutes), but that is probably hard to avoid. Two comments:

  • use your real name for the "Authors" field
  • the documentation line starting with "An example with" should be indented

After fixing those, feel free to set this to positive review.

comment:3 Changed 7 years ago by cremona

Please try to find a shorter doctest! Even when tagged with "long time" 4 minutes is going to add quite a lot to the testing time.

But thanks for doing this, anyway.

comment:4 Changed 7 years ago by git

  • Commit changed from 2085e0c096a1a18a15d8dbcdde06627749b90542 to 6a54aa459703a03350952ae22cedd8d3f190ce73

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

6a54aa4Fixed indentation problem in doctest.

comment:5 Changed 7 years ago by git

  • Commit changed from 6a54aa459703a03350952ae22cedd8d3f190ce73 to 522aa2f9b87a59d22acc1cb8a2ab66dec808fab5

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

522aa2fFound an example for the doctest that takes much less to terminate (about 4s).

comment:6 Changed 7 years ago by mmasdeu

  • Authors changed from mmasdeu to Marc Masdeu
  • Status changed from needs_work to positive_review

comment:7 Changed 7 years ago by pbruin

This long example, and the special lim1 and limtriv in your last commit, make it sound like a good moment to look into #9322, which should generally speed up simon_two_descent() by changing the parameters.

comment:8 Changed 7 years ago by pbruin

The old example would be more than twice as fast with Simon's default parameters:

sage: %timeit -n 1 -r 1 E.rank(lim1=2,lim3=4,limtriv=2)
1 loops, best of 1: 1min 42s per loop

So indeed fixing #9322 would help.

Another thing: E.torsion_subgroup() fails as well, I've opened ticket #16011 for it.

comment:9 Changed 7 years ago by pbruin

  • Status changed from positive_review to needs_work

The doctest in the last commit turns out to break (return an incorrect result) after #11005. Marc and I could reproduce this using Simon's script inside gp. We will mark the doctest with # known bug and report the bug to Denis Simon.

comment:10 Changed 7 years ago by pbruin

  • Dependencies set to #16022

comment:11 Changed 7 years ago by git

  • Commit changed from 522aa2f9b87a59d22acc1cb8a2ab66dec808fab5 to 8bceb36e55fe51e86815f8432eec201d401595ea

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

db79035Merge branch 'develop' into gp_simon_relative
a92a80eChanged the doctest to make it independent of variable output.
275e4befix a bug in Denis Simon's 2-descent program
8bceb36Merge branch 'u/pbruin/16022-simon_two_descent_bug' of trac.sagemath.org:sage into gp_simon_relative

comment:12 Changed 7 years ago by mmasdeu

  • Status changed from needs_work to needs_review

I merged #16022 and now the it passes the doctests.

comment:13 Changed 7 years ago by pbruin

  • Status changed from needs_review to positive_review

comment:14 Changed 7 years ago by vbraun

  • Branch changed from u/mmasdeu/gp_simon_relative to 8bceb36e55fe51e86815f8432eec201d401595ea
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.