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

Update Simon's GP scripts #11005

Closed
JohnCremona opened this issue Mar 23, 2011 · 55 comments
Closed

Update Simon's GP scripts #11005

JohnCremona opened this issue Mar 23, 2011 · 55 comments

Comments

@JohnCremona
Copy link
Member

[See #15608 for a list of open simon_two_descent tickets]

Denis Simon has new versions of his scripts (see http://www.math.unicaen.fr/~simon/), these should be updated (see src/ext/pari/simon).

Depends on #11230
Depends on #11234
Depends on #11130
Depends on #15483

CC: @williamstein @pjbruin

Component: elliptic curves

Keywords: simon_two_descent spkg

Author: Peter Bruin

Branch/Commit: 24b6fe9

Reviewer: Jeroen Demeyer

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

@JohnCremona JohnCremona added this to the sage-5.0 milestone Mar 23, 2011
@JohnCremona JohnCremona self-assigned this Mar 23, 2011
@JohnCremona
Copy link
Member Author

Attachment: trac_11005-simon-sage.patch.gz

applies to 4.7.alpha1

@JohnCremona
Copy link
Member Author

applies to 4.7.alpha1

@JohnCremona

This comment has been minimized.

@JohnCremona
Copy link
Member Author

comment:1

Attachment: trac_11005-simon-extcode.patch.gz

@JohnCremona
Copy link
Member Author

comment:2

Note to reviewers. It is very likely that you will find elliptic curves where the scripts do not work (I have some of my own). That is not a reason to reject this enhancement. The scripts are not perfect, but they are all we have over number fields! As a result of this ticket, we get both updated version of the scripts (with some bug-fixes and enhancements) and also faster execution. But not perfection.

@JohnCremona
Copy link
Member Author

comment:3

Not yet ready for review: still testing.

@JohnCremona
Copy link
Member Author

Attachment: simon-20110321.spkg.gz

New spkg

@sagetrac-mraum

This comment has been minimized.

@sagetrac-mraum
Copy link
Mannequin

sagetrac-mraum mannequin commented Mar 25, 2011

comment:5

I created a new spkg and I updated the way to update import the library, removing one GP-script from the spkg.
I can give this a positive review, but before someone needs to test this on Mac at least.

@sagetrac-mraum

This comment has been minimized.

@sagetrac-mraum
Copy link
Mannequin

sagetrac-mraum mannequin commented Mar 25, 2011

comment:6

Attachment: trac-11005-simon-sage-2.patch.gz

@JohnCremona
Copy link
Member Author

comment:7

Martin, the spkg at http://sage.math.washington.edu/home/mraum/simon-20110321.spkg seems to be identical to my original one, which cannot be right.

@JohnCremona
Copy link
Member Author

Changed keywords from none to simon

@JohnCremona
Copy link
Member Author

Changed author from John Cremona to John Cremona, Martin Raum

@sagetrac-mraum
Copy link
Mannequin

sagetrac-mraum mannequin commented Apr 22, 2011

comment:8

I checked this, but indeed this is the right package. The point was to update the spkg-install script. The rest of the spkg, I think is perfect. Well, we should incorporate the new scripts that you received by Simon.

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Feb 21, 2014

Changed branch from u/pbruin/11005-Simon_update to 24b6fe9

@vbraun
Copy link
Member

vbraun commented Feb 23, 2014

Changed commit from 24b6fe9 to none

@vbraun vbraun reopened this Feb 23, 2014
@pjbruin
Copy link
Contributor

pjbruin commented Feb 23, 2014

Changed dependencies from #11230, #11234, #11130 to #11230, #11234, #11130, #15483

@jdemeyer
Copy link

comment:38

The pbori.pyx failure is from #15755.

@jdemeyer
Copy link

comment:39

Why the dependency on #15483? Now both these tickets depend on eachother. I would prefer to fix #11005 first (using # 32-bit/# 64-bit) and then do #15483 on top of this.

@jdemeyer
Copy link

Commit: 24b6fe9

@jdemeyer
Copy link

Changed branch from 24b6fe9 to u/pbruin/11005-Simon_update

@jdemeyer
Copy link

New commits:

24b6fe9Update Denis Simon's GP scripts to versions of 06/04/2011 (ell.gp)

@pjbruin
Copy link
Contributor

pjbruin commented Feb 24, 2014

comment:42

Replying to @jdemeyer:

Why the dependency on #15483?

Because #15483 is the obvious fix for the problem from comment:36.

Now both these tickets depend on eachother. I would prefer to fix #11005 first (using # 32-bit/# 64-bit) and then do #15483 on top of this.

I thought Git was supposed to make things like this easy. I knew that I made the two tickets depend on each other, of course, and agree that circular dependencies are in abstracto not very nice. However, in this case, what looks like a circular dependency is just a way of saying that the tickets should be merged together; the structure of the Git branches is still linear and easy to understand. Moreover, Git will automatically give the same result independently of the ordering in which the two branches are merged. I don't feel like spending any time on making each of the two tickets pass doctests separately.

I also realise you don't like the patch at #15483, so I will make a bit more propaganda for it.

@vbraun
Copy link
Member

vbraun commented Feb 27, 2014

Changed branch from u/pbruin/11005-Simon_update to 24b6fe9

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