Opened 7 years ago

Closed 7 years ago

#1239 closed enhancement (fixed)

[with patch, with positive review *BUT* see comments] Wrap Simon's new gp two descent code

Reported by: robertwb Owned by: robertwb
Priority: major Milestone: sage-2.9
Component: number theory Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Scripts were recently updated http://www.math.unicaen.fr/~simon/

It now handles two-torsion more uniformly, works on more curves, and actually returns points on the curve given.

Attachments (5)

extcode_simon_code.patch (193.3 KB) - added by robertwb 7 years ago.
simon-interface.hg (7.5 KB) - added by robertwb 7 years ago.
trac-1239.patch (4.4 KB) - added by was 7 years ago.
tentative_trac-1239.patch
1239-docstring-issues.patch (2.8 KB) - added by robertwb 7 years ago.
1239-integrality-issues.patch (2.5 KB) - added by robertwb 7 years ago.

Download all attachments as: .zip

Change History (17)

Changed 7 years ago by robertwb

Changed 7 years ago by robertwb

comment:1 Changed 7 years ago by robertwb

  • Owner changed from was to robertwb

John Cremona and I worked on this during Sage Days 6.

The attached patches have the new version of the code (to be applied to extcode) and a revised interface.

This also includes an implementation of transformations between different Weierstrass models.

comment:2 Changed 7 years ago by robertwb

  • Summary changed from Wrap Simon's new gp two descent code to [with patch] Wrap Simon's new gp two descent code

comment:3 Changed 7 years ago by mabshoff

  • Milestone set to sage-2.8.14

comment:4 Changed 7 years ago by was

  • Summary changed from [with patch] Wrap Simon's new gp two descent code to [with patch, with negative review] Wrap Simon's new gp two descent code

WARNING: This is full of bugs and issues.

E.g.,

sage: E = EllipticCurve([0, 0, 1/216, -7/1296, 1/7776])
sage: F = E.global_integral_model(); F
outputs a non-integral model!

DO NOT apply this until further patche(s) are posted.

I'm working on some now.

ALSO -- there are many new functions with no doctets.

comment:5 Changed 7 years ago by was

Some missing doctests or things that will cause latex problems:

a/sage/schemes/elliptic_curves/ell_generic.py
integral_model
change_weierstrass_model

a/sage/rings/complex_double.pyx
argument

* number_field_element.pyx -- nth_root has \ but no r"""
* same prob in WeierstrassIsomorphism
* no doctest in init method for WeierstrassIsomorphism
* no doctest in init method for WeierstrassIsomorphism _call_

comment:6 Changed 7 years ago by was

[11:14pm] wstein-rvw-1239: It might be easy for you to fix the problems.
[11:14pm] wstein-rvw-1239: E.g.,            sage: E = EllipticCurve([0, 0, 1/216, -7/1296, 1/7776])
[11:14pm] wstein-rvw-1239:             sage: F = E.global_integral_model(); F
[11:14pm] wstein-rvw-1239: doesn't return an integral model.
[11:14pm] wstein-rvw-1239: E = EllipticCurve([1/3, 5]); E
[11:14pm] wstein-rvw-1239: E.integral_model()
[11:14pm] wstein-rvw-1239: returns a non-integral model

Changed 7 years ago by was

tentative_trac-1239.patch

comment:7 Changed 7 years ago by was

[good review -- on extcode] The extcode bundle is *OK* -- no problems.

Changed 7 years ago by robertwb

comment:8 Changed 7 years ago by robertwb

  • Status changed from new to assigned

The global_integral_model / integral_model code in question is John Cremona's. I'll look into it more.

WARNING: The extcode patch can't go in without this one (due to interface changes).

comment:9 Changed 7 years ago by was

  • Summary changed from [with patch, with negative review] Wrap Simon's new gp two descent code to [with patch, with positive review *BUT* see comments] Wrap Simon's new gp two descent code

Changed 7 years ago by robertwb

comment:10 Changed 7 years ago by robertwb

Turned out to be an indentation issue. Also added another doctest.

Should be ready to go in now.

comment:11 Changed 7 years ago by mabshoff

Merged in 2.9.rc0.

comment:12 Changed 7 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.