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 Cremona's tables for Sage #11587

Closed
williamstein opened this issue Jul 11, 2011 · 90 comments
Closed

update Cremona's tables for Sage #11587

williamstein opened this issue Jul 11, 2011 · 90 comments

Comments

@williamstein
Copy link
Contributor

John Cremona tells me that: "All data for elliptic curves of conductors from 130k to 200k is at
http://www.warwick.ac.uk/staff/J.E.Cremona/ftp/data/ and also as a tar
file at http://www.warwick.ac.uk/staff/J.E.Cremona/ftp/ecdata-2011-10-26.tgz
-- not to mention at http://code.google.com/p/ecdata/ !"

The goal of this ticket is to:

(1) Create a new drop-in-replacement sqlite database that has the data up to level 10000, which will be included standard with Sage.

(2) Create a new sqlite database that has the data up to level 200000, which will be an optional spkg.

This is complicated because the current packages in Sage use ZODB. Also, Cremona's database format has changed somewhat.

There is now a patch that depends upon #11642 (which depends upon #11640).

You can find updated elliptic_curve and database_cremona_ellcurve spkgs at http://wstein.org/home/ohanar/cremona-database/.


New spkg: http://sage.math.washington.edu/home/leif/Sage/spkgs/elliptic_curves-0.3.spkg

Apply attachment: trac_11587.patch to the Sage library.

Apply attachment: trac_11587-docfixes.patch to the Sage library.

Apply attachment: trac_11587-rmhardcode.proper.patch to the Sage library.

Apply attachment: trac_11587-fix_speed_regression.patch to the Sage library.

Apply attachment: trac_11587-depfix.patch to Sage root.

Optional spkg: http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20111029.spkg

Depends on #11642

CC: @loefflerd

Component: elliptic curves

Keywords: elliptic curves ellcurve database

Author: R. Andrew Ohana

Reviewer: John Cremona, Tom Boothby

Merged: sage-4.8.alpha0

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

@ohanar
Copy link
Member

ohanar commented Jul 11, 2011

comment:1

Updated description to reflect the current status of the tables.

@ohanar

This comment has been minimized.

@JohnCremona
Copy link
Member

comment:2

Note: only the allgens file format has changed.

@ohanar

This comment has been minimized.

@ohanar
Copy link
Member

ohanar commented Aug 2, 2011

comment:3

There are new spkgs at
http://wstein.org/home/ohanar/cremona-database

@JohnCremona
Copy link
Member

Reviewer: John Cremona

@JohnCremona
Copy link
Member

comment:5

I have a few questions. Also since I do not know SQLITE and do not have the time to learn it just for this, the code will need to be reviewed by someone else.

There are two spkgs at the link, and one patch on this ticket. Please can you explain how they are related? Is one of the spkgs intended to be part of the standard Sage distribution so that every copy of Sage has the mini-database, as is the case now? And is the other one a replacement for the optional large database?

Secondly, you warn that the database takes some time to create. Is this once-only time which you had to spend in creating the database which is part of the spkg, or is it time which must be spent by anyone installing the large optional spkg?

To test this, I assume that the following steps are necessary; please correct me if I am wrong.

  1. apply patches at Rewrite/improve/fix SQLDatabase and SQLQuery objects #11642
  2. apply the patch here (trac_11587.patch)
  3. sage -b
  4. sage -i http://wstein.org/home/ohanar/cremona-database/elliptic_curves-0.2.spkg
  5. sage -b, test everything works
  6. sage -i http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20110801.spkg
  7. sage -b, test everything still works

Lastly: I expect that at regular intervals over the next few months I will be releasing updates to the database (for example, the range 170k=180k is about 70% done already). Please leave rather clear instructions for what to do for future updates; in particular, will it be neceaary to rebuild everything, or just to read in the new files (e.g. *.170000-179999 next time)?

@ohanar

This comment has been minimized.

@ohanar
Copy link
Member

ohanar commented Aug 2, 2011

comment:6

Replying to @JohnCremona:

There are two spkgs at the link, and one patch on this ticket. Please can you explain how they are related?

The patch updates sage.databases.cremona to wrap the new SQLite databases. It provides about the same functionality as the old ZODB (there are some slight modifications to actually reflect the documentation, which wasn't respected before -- specifically the mini-database only has data from allcurves).

Is one of the spkgs intended to be part of the standard Sage distribution so that every copy of Sage has the mini-database, as is the case now? And is the other one a replacement for the optional large database?

Correct. The elliptic_curve spkg is included every copy of sage, and includes both the mini-database, as well as a very small database of curves William finds interesting. The database_cremona_ellcurve spkg includes (most of) the data from allcurves, allgens, allbsd, and degphi for every curve that you have listed.

Secondly, you warn that the database takes some time to create. Is this once-only time which you had to spend in creating the database which is part of the spkg, or is it time which must be spent by anyone installing the large optional spkg?

Currently the spkgs simply install pre-built SQL databases, so currently this is a once-only thing. That said, on my home system it only takes ~4mins to create the database from scratch.

To test this, I assume that the following steps are necessary; please correct me if I am wrong.

  1. apply patches at Rewrite/improve/fix SQLDatabase and SQLQuery objects #11642
  2. apply the patch here (trac_11587.patch)
  3. sage -b
  4. sage -i http://wstein.org/home/ohanar/cremona-database/elliptic_curves-0.2.spkg
  5. sage -b, test everything works
  6. sage -i http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20110801.spkg
  7. sage -b, test everything still works

Yes, that looks correct. The only thing to be aware of is that #11642 depends on #11640 (which is just a small patch), and that there are optional doctests you should run in step 7.

Lastly: I expect that at regular intervals over the next few months I will be releasing updates to the database (for example, the range 170k=180k is about 70% done already). Please leave rather clear instructions for what to do for future updates; in particular, will it be neceaary to rebuild everything, or just to read in the new files (e.g. *.170000-179999 next time)?

Currently there is simple script to rebuild everything, but (in theory) it is also possible to simply add the new files. The former:

rm SAGE_ROOT/data/cremona/cremona.db
SAGE_ROOT/sage
sage: sage.databases.cremona.build('cremona','/path/to/ecdata.tgz')

the later should be (although I haven't tested it):

make a directory that only contains the new files
SAGE_ROOT/sage
sage: cData = sage.databases.cremona.LargeCremonaDatabase('cremona',False)
sage: cData._init_from_ftpdata('/path/to/said/directory')

That said, I've marked myself as the maintainer of the database_cremona_ellcurve spkg, so I'll try to keep it up to date (of course it would help if you let me know when you update your tables of course (: ).

@ohanar

This comment has been minimized.

@ohanar
Copy link
Member

ohanar commented Aug 10, 2011

comment:7

I've updated the database_cremona_ellcurve spkg to include the latest tables, which can still be found at http://wstein.org/home/ohanar/cremona-database (the old one is still there there as well).

I'm also attaching an updated patch that mainly updates the documentation to reflect the current state of the tables (upper bound of 180,000 vs 130,000), as well as the spkg version. It also limits the outputs of a few queries where only the first entry is needed.

@ohanar
Copy link
Member

ohanar commented Aug 24, 2011

fixes the broken doctest for cremona_mini

@boothby
Copy link

boothby commented Aug 25, 2011

comment:8

Attachment: trac_11587.patch.gz

looks good, works good

@boothby
Copy link

boothby commented Aug 25, 2011

Author: Andrew Ohana

@boothby
Copy link

boothby commented Aug 25, 2011

Changed reviewer from John Cremona to John Cremona, Tom Boothby

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 8, 2011

Dependencies: #11642

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 8, 2011

comment:9

That's +1.4 MB, or +78% for the standard spkg.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 8, 2011

Changed keywords from none to elliptic curves ellcurve database

@JohnCremona
Copy link
Member

comment:51

Replying to @nexttime:

Replying to @JohnCremona:

Any effort in removing this upgrade which takes more than the few minutes it would take to fix the very minor problem in it would be a complete waste of time.

I fully agree, but unfortunately it's relatively easy to "unmerge" this ticket, and it's in general not up to the release manager to fix such things (that go beyond simple rebasing, fixing noise, permissions or commit messages etc., which IMHO at least partially already is a voluntary service).

Of course -- I just meant that you should do the absolute minimum and not spend effort making the old system more clever. And I just have not had time to fix the patch.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Oct 16, 2011

apply over previous patches

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Oct 16, 2011

comment:52

Attachment: trac_11587-fix_speed_regression.patch.gz

@loefflerd

This comment has been minimized.

@loefflerd loefflerd mannequin added the s: needs review label Oct 16, 2011
@JohnCremona
Copy link
Member

comment:53

Replying to @loefflerd:

Not tested but David's patch looks perfect and I believe his comment. Better if someone else test this as David and I use the same machine.

If this is not merged soon it will become out of date since I have currently reached 198800 and expect to release an extension up to 200k within a week!

@ohanar
Copy link
Member

ohanar commented Oct 30, 2011

comment:54

Everything works as expected.

@loefflerd
I don't think that those lines of code were actually ever used in the old version, since the small database actually included the generators despite the documentation saying otherwise (the new version follows what the documentation specified).

@JohnCremona
I've posted a new spkg with your expanded tables, same location as before.

@JohnCremona
Copy link
Member

comment:55

Replying to @ohanar:

Everything works as expected.

@loefflerd
I don't think that those lines of code were actually ever used in the old version, since the small database actually included the generators despite the documentation saying otherwise (the new version follows what the documentation specified).

@JohnCremona
I've posted a new spkg with your expanded tables, same location as before.

Many thanks to all. As predicted, the database was updated to go up to 200k a few days ago.

@jdemeyer
Copy link

Merged: sage-4.7.3.alpha0

@jdemeyer

This comment has been minimized.

@JohnCremona
Copy link
Member

comment:58

I just updated the description to reflect the fact that the optional db now goes up to 200k.

@JohnCremona

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Nov 3, 2011

comment:59

Replying to @ohanar:

I've posted a new spkg with your expanded tables, same location as before.

Not really same location as before, I updated the ticket description.

@jdemeyer
Copy link

jdemeyer commented Nov 3, 2011

Milestone sage-4.7.3 deleted

@jdemeyer jdemeyer removed this from the sage-4.8 milestone Nov 3, 2011
@jdemeyer
Copy link

jdemeyer commented Nov 3, 2011

Changed merged from sage-4.7.3.alpha0 to sage-4.8.alpha0

@jdemeyer jdemeyer added this to the sage-4.8 milestone Nov 3, 2011
@ohanar
Copy link
Member

ohanar commented Nov 27, 2011

comment:62

I've posted an updated spkg based on Cremona's update to 210000 (http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20111121.spkg).

I noticed that I never put in a way to install the old spkg for old copies of sage, so I added a recursive sage -i for now, until we have a standard that frees the user from downloading two different versions of an spkg. It should work for any version of sage released within the last year, although I haven't tested it super thoroughly (it works with stable 4.7.x, 4.8.alpha3, and 4.3.3.alpha0).

@jdemeyer
Copy link

comment:63

Replying to @ohanar:

I've posted an updated spkg based on Cremona's update to 210000 (http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20111121.spkg).

Please make a new ticket for making this an optional spkg (this ticket is closed).

@ohanar
Copy link
Member

ohanar commented Nov 28, 2011

comment:64

Replying to @jdemeyer:

Please make a new ticket for making this an optional spkg (this ticket is closed).

Sure, my only reason for posting it in this ticket was due to the unaddressed issue of dealing with old copies of Sage.

@JohnCremona
Copy link
Member

comment:65

Doing a full test after installing the new optional database (onto 4.8.alpha4) I find one doctest failure in devel/sage/doc/en/bordeaux_2008/elliptic_curves.rst:

    sage: E = EllipticCurve([1,2,5,7,17])
    sage: E.conductor()       # not in the Tables
    154907
    sage: E.gens()            # a few seconds
    [(1 : 3 : 1), (67/4 : 507/8 : 1)]

since that curve is now in the database and the listed generators are different. I suggest instead

sage: E = EllipticCurve([0,0,1,-19,60])
sage: E.conductor()        # not in the Tables
1129211
sage: E.gens()             # a few seconds
[(-5 : 5 : 1), (-4 : 8 : 1), (-15/4 : 67/8 : 1), (-3 : 9 : 1)]

Should the ticket be reopened -- this is not an issue in testing without the optional db installed?

@JohnCremona
Copy link
Member

comment:66

Replying to @JohnCremona:

Should the ticket be reopened -- this is not an issue in testing without the optional db installed?

See #12184

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

5 participants