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

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

Closed
robertwb opened this issue Nov 21, 2007 · 11 comments

Comments

@robertwb
Copy link
Contributor

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.

Component: number theory

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

@robertwb
Copy link
Contributor Author

Attachment: extcode_simon_code.patch.gz

@robertwb
Copy link
Contributor Author

comment:1

Attachment: simon-interface.hg.gz

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.

@robertwb robertwb assigned robertwb and unassigned williamstein Nov 21, 2007
@sagetrac-mabshoff sagetrac-mabshoff mannequin added this to the sage-2.8.14 milestone Nov 21, 2007
@williamstein
Copy link
Contributor

comment:4

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.

@williamstein williamstein changed the title Wrap Simon's new gp two descent code [with negative review] Wrap Simon's new gp two descent code Dec 15, 2007
@williamstein
Copy link
Contributor

comment:5

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_

@williamstein
Copy link
Contributor

comment:6
[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

@williamstein
Copy link
Contributor

Attachment: trac-1239.patch.gz

tentative_trac-1239.patch

@williamstein
Copy link
Contributor

comment:7

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

@robertwb
Copy link
Contributor Author

Attachment: 1239-docstring-issues.patch.gz

@robertwb
Copy link
Contributor Author

comment:8

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).

@williamstein williamstein changed the title [with negative review] Wrap Simon's new gp two descent code [with patch, with positive review *BUT* see comments] Wrap Simon's new gp two descent code Dec 15, 2007
@robertwb
Copy link
Contributor Author

comment:10

Attachment: 1239-integrality-issues.patch.gz

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

Should be ready to go in now.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Dec 15, 2007

comment:11

Merged in 2.9.rc0.

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Dec 15, 2007
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

2 participants