#11587 closed enhancement (fixed)
update Cremona's tables for Sage
Reported by: | was | Owned by: | cremona |
---|---|---|---|
Priority: | major | Milestone: | sage-4.8 |
Component: | elliptic curves | Keywords: | elliptic curves ellcurve database |
Cc: | davidloeffler | Merged in: | sage-4.8.alpha0 |
Authors: | R. Andrew Ohana | Reviewers: | John Cremona, Tom Boothby |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #11642 | Stopgaps: |
Description (last modified by )
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 trac_11587.patch to the Sage library.
Apply trac_11587-docfixes.patch to the Sage library.
Apply trac_11587-rmhardcode.proper.patch to the Sage library.
Apply trac_11587-fix_speed_regression.patch to the Sage library.
Apply trac_11587-depfix.patch to Sage root.
Optional spkg: http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20111029.spkg
Attachments (6)
Change History (72)
comment:1 Changed 11 years ago by
- Description modified (diff)
comment:2 Changed 11 years ago by
Note: only the allgens file format has changed.
comment:3 Changed 11 years ago by
- Description modified (diff)
There are new spkgs at http://wstein.org/home/ohanar/cremona-database
comment:4 Changed 11 years ago by
- Status changed from new to needs_review
comment:5 follow-up: ↓ 6 Changed 11 years ago by
- Reviewers set to John Cremona
- Status changed from needs_review to needs_info
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.
- apply patches at #11642
- apply the patch here (trac_11587.patch)
- sage -b
- sage -i http://wstein.org/home/ohanar/cremona-database/elliptic_curves-0.2.spkg
- sage -b, test everything works
- sage -i http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20110801.spkg
- 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)?
comment:6 in reply to: ↑ 5 Changed 11 years ago by
- Description modified (diff)
- Status changed from needs_info to needs_review
Replying to cremona:
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.
- apply patches at #11642
- apply the patch here (trac_11587.patch)
- sage -b
- sage -i http://wstein.org/home/ohanar/cremona-database/elliptic_curves-0.2.spkg
- sage -b, test everything works
- sage -i http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20110801.spkg
- 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 (: ).
comment:7 Changed 11 years ago by
- Description modified (diff)
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.
comment:8 Changed 11 years ago by
- Reviewers changed from John Cremona to John Cremona, Tom Boothby
- Status changed from needs_review to positive_review
looks good, works good
comment:9 follow-up: ↓ 10 Changed 11 years ago by
- Dependencies set to #11642
- Description modified (diff)
- Keywords elliptic curves ellcurve database added
That's +1.4 MB, or +78% for the standard spkg.
comment:10 in reply to: ↑ 9 Changed 11 years ago by
Replying to leif:
That's +1.4 MB, or +78% for the standard spkg.
I can rewrite the install script to simply create the database from Cremona's text files. On my system it takes ~6sec to build it from scratch, and would probably decrease the size of the spkg to a bit over 1MB.
comment:11 follow-up: ↓ 12 Changed 11 years ago by
- Description modified (diff)
I've created a new spkg that builds the cremona_mini database from Cremona's original text file. This requires a patch to the Sage root repository to reflect the new dependencies. I'm going to keep the optional spkg the same since it takes >5mins to build the database.
comment:12 in reply to: ↑ 11 ; follow-up: ↓ 13 Changed 11 years ago by
Replying to ohanar:
I've created a new spkg that builds the cremona_mini database from Cremona's original text file. This requires a patch to the Sage root repository to reflect the new dependencies.
Cool, thanks.
I'm going to keep the optional spkg the same since it takes >5mins to build the database.
Optional is optional; I'm not aware of any size demands or restrictions there...
comment:13 in reply to: ↑ 12 Changed 11 years ago by
Replying to leif:
Replying to ohanar:
I'm going to keep the optional spkg the same since it takes >5mins to build the database.
Optional is optional; I'm not aware of any size demands or restrictions there...
True, but I realized when updating the spkg that creating the database from text files more closely follows the spkg design of building libraries from source.
comment:14 follow-up: ↓ 15 Changed 11 years ago by
Many thanks for your work on this, Andrew. It would be good if it were very easy to extend further (say just by having more data files in the right place on installation), since I have not stopped and in fact am very close to adding the range 180k=190k. Adding that data (and more) should now be just a matter of adding some data files to the spkg, right? Is the information such as the maximum conductor hard-wired into the code or just read from the database after construction from raw data files?
comment:15 in reply to: ↑ 14 ; follow-up: ↓ 16 Changed 11 years ago by
Replying to cremona:
It would be good if it were very easy to extend further (say just by having more data files in the right place on installation), since I have not stopped and in fact am very close to adding the range 180k=190k. Adding that data (and more) should now be just a matter of adding some data files to the spkg, right?
Currently the spkg includes basically a single file -- a prebuilt database. This is primarily because it is much faster to have the package copy the file, rather than build the database from scratch (I was going to use time it takes to create it on sage.math as an example, but I got too impatient to let it finish). The script to create this database is sage.databases.cremona.build
, this will build and install the database from your tgz file.
If we decide to, we can move the build script to be inside the spkg, with the knowledge that we should not use it on slow filesystems. If we do so, we should only include allcurves, allgens, allbsd, and degphi since those are the only files sage currently uses.
Is the information such as the maximum conductor hard-wired into the code or just read from the database after construction from raw data files?
There is still a bit of hard-wired code: a bit in the documentation, the build script, and an error message. It is possible to remove all of these references, the only reason I didn't when working on this patch was because I wanted to get sage up to speed. It is a simple fix, although if we decide remove the build scripts from the sage library to the spkg instead, we might as well do this at the same time.
comment:16 in reply to: ↑ 15 Changed 11 years ago by
Replying to ohanar:
Replying to cremona:
It would be good if it were very easy to extend further (say just by having more data files in the right place on installation), since I have not stopped and in fact am very close to adding the range 180k=190k. Adding that data (and more) should now be just a matter of adding some data files to the spkg, right?
Currently the spkg includes basically a single file -- a prebuilt database. This is primarily because it is much faster to have the package copy the file, rather than build the database from scratch (I was going to use time it takes to create it on sage.math as an example, but I got too impatient to let it finish). The script to create this database is
sage.databases.cremona.build
, this will build and install the database from your tgz file.If we decide to, we can move the build script to be inside the spkg, with the knowledge that we should not use it on slow filesystems. If we do so, we should only include allcurves, allgens, allbsd, and degphi since those are the only files sage currently uses.
Is the information such as the maximum conductor hard-wired into the code or just read from the database after construction from raw data files?
There is still a bit of hard-wired code: a bit in the documentation, the build script, and an error message. It is possible to remove all of these references, the only reason I didn't when working on this patch was because I wanted to get sage up to speed. It is a simple fix, although if we decide remove the build scripts from the sage library to the spkg instead, we might as well do this at the same time.
Thanks for the explanation. I agree that it is good to have the database file itself in the package and not the raw data, as long as it (the spkg) also has instructions on how to (re)create this from my raw data. And a note of the precise things to be changed when the range is extended would also be a good idea, even if that's only a couple of places. I'm thinking that it may not be you doing this next time!
comment:17 follow-up: ↓ 22 Changed 11 years ago by
If 190k is almost done, I can add the upgrading instructions then.
comment:18 Changed 11 years ago by
I get warnings from Sphinx:
.../local/lib/python2.6/site-packages/sage/databases/cremona.py:docstring of sage.databases.cremona:26: (WARNING/2) Definition list ends without a blank line; unexpected unindent. .../local/lib/python2.6/site-packages/sage/databases/cremona.py:docstring of sage.databases.cremona:36: (WARNING/2) Definition list ends without a blank line; unexpected unindent.
comment:19 follow-up: ↓ 20 Changed 11 years ago by
I'm not sure to which ticket this belongs, but I get an apparently trivial doctest error in the following example of doc/en/bordeaux_2008/elliptic_curves.rst
:
To use the database, just create a curve by giving :: sage: EllipticCurve('5077a1') Elliptic Curve defined by y^2 + y = x^3 - 7*x + 6 over Rational Field sage: C = CremonaDatabase() sage: C.number_of_curves() #optional - cremona 847550 sage: C[37] {'a': {'a1': [[0, 0, 1, -1, 0], 1, 1], 'b1': [[0, 1, 1, -23, -50], 0, 3], ... sage: C.isogeny_class('37b') [Elliptic Curve defined by y^2 + y = x^3 + x^2 - 23*x - 50 over Rational Field, ...]
File ".../doc/en/bordeaux_2008/elliptic_curves.rst", line 22: sage: C[37] Expected: {'a': {'a1': [[0, 0, 1, -1, 0], 1, 1], 'b1': [[0, 1, 1, -23, -50], 0, 3], ... Got: {'allcurves': {'a1': [[0, 0, 1, -1, 0], 1, 1], 'b1': [[0, 1, 1, -23, -50], 0, 3], 'b2': [[0, 1, 1, -1873, -31833], 0, 1], 'b3': [[0, 1, 1, -3, 1], 0, 3]}}
(Just the first key differs; a
vs. allcurves
.)
comment:20 in reply to: ↑ 19 ; follow-up: ↓ 21 Changed 11 years ago by
- Status changed from positive_review to needs_work
Replying to leif:
I'm not sure to which ticket this belongs, but I get an apparently trivial doctest error in the following example of
doc/en/bordeaux_2008/elliptic_curves.rst
:
(Just the first key differs;
a
vs.allcurves
.)
I'm sure this is the correct ticket since the database has changed. It's easy to forget to test this part of the documentation. Changing the expected output to the actual output would be fine. Hence back to "Needs (trivial) work".
comment:21 in reply to: ↑ 20 Changed 11 years ago by
Replying to cremona:
Replying to leif:
I'm not sure to which ticket this belongs, but I get an apparently trivial doctest error in the following example of
doc/en/bordeaux_2008/elliptic_curves.rst
:(Just the first key differs;
a
vs.allcurves
.)I'm sure this is the correct ticket since the database has changed. It's easy to forget to test this part of the documentation. Changing the expected output to the actual output would be fine.
Yup, didn't test this, so a good catch (and yes, it does belong here, I made the change in key names). Also, if you run the optional test, that will fail as well (we may want to scrap it for something else, since it adds to the maintenance of the optional spkg).
comment:22 in reply to: ↑ 17 Changed 11 years ago by
Replying to ohanar:
If 190k is almost done, I can add the upgrading instructions then.
It's almost done in the sense that I have everything except that 3 curves in the range are "missing" on account of some difficulties with modular symbols which I will now try to fix (requires revising code and testing). Either I'll have this by the end of Thursday, or I'll be away for a week and it will take longer.
comment:23 Changed 11 years ago by
- Description modified (diff)
comment:24 Changed 11 years ago by
- Status changed from needs_work to needs_review
comment:25 follow-up: ↓ 26 Changed 11 years ago by
Applying trac_11587.patch onto 4.7.2.alpha1 failed. And before that, installing the new spkg using sage -i ended with
... Thread model: posix gcc version 4.4.3 (Ubuntu 4.4.3-4ubuntu5) **************************************************** /home/jec/sage-4.7.2.alpha1.ecdb/local/bin/sage-sage: line 807: 12145 Killed tee -a ../install.log "$SAGE_LOGS/$PKG_NAME".log
which looks like an error message, I am not sure. Am I doing something wrong?
comment:26 in reply to: ↑ 25 ; follow-up: ↓ 27 Changed 11 years ago by
Replying to cremona:
Applying trac_11587.patch onto 4.7.2.alpha1 failed.
Did you first apply #11640 and #11642?
And before that, installing the new spkg using sage -i ended with
... Thread model: posix gcc version 4.4.3 (Ubuntu 4.4.3-4ubuntu5) **************************************************** /home/jec/sage-4.7.2.alpha1.ecdb/local/bin/sage-sage: line 807: 12145 Killed tee -a ../install.log "$SAGE_LOGS/$PKG_NAME".log
which looks like an error message, I am not sure.
Yes. No idea what might have caused that.
comment:27 in reply to: ↑ 26 Changed 11 years ago by
comment:28 Changed 11 years ago by
OK, with #11640 and #11642 applied first the patches to the Sage library apply fine. Dor the deps one I just manually edited the file since I could find no trace of mercurial in $SAGE_ROOT/spkgs/standard (or at least hg gave an error message). Then I rebuilt everything with sage -ba, and I am now doing a make ptestlong, and will report back.
Small comment, since I know that Andrew will be updating the conductor ranges anyway: at the moment smallest_conductor() returns 1 which will surprise some people as the actual smallest is 11!
comment:29 follow-up: ↓ 30 Changed 11 years ago by
- Status changed from needs_review to positive_review
All test pass, so I am giving this a positive review. This is for the default database (max conductor 10000); but this ticket cannot be merged until the large database is also ready to go and has been tested.
comment:30 in reply to: ↑ 29 ; follow-up: ↓ 32 Changed 11 years ago by
Replying to cremona:
All test pass, so I am giving this a positive review.
I didn't expect something else. ;-)
This is for the default database (max conductor 10000); but this ticket cannot be merged until the large database is also ready to go and has been tested.
Which will be when?
Can't we just merge this ticket and open a follow-up in case something goes wrong with the larger (anyway optional) database?
comment:31 Changed 11 years ago by
P.S.: deps
is part of the "root" repository, which is located in $SAGE_ROOT
, so you have to apply the patch there.
comment:32 in reply to: ↑ 30 ; follow-up: ↓ 33 Changed 11 years ago by
Replying to leif:
Replying to cremona:
All test pass, so I am giving this a positive review.
I didn't expect something else. ;-)
This is for the default database (max conductor 10000); but this ticket cannot be merged until the large database is also ready to go and has been tested.
Which will be when?
I finished creating the data this morning, and Andrew Ohana is updating his optional spkg (?) now.
Can't we just merge this ticket and open a follow-up in case something goes wrong with the larger (anyway optional) database?
No, I don't think so, since the database has completely changed and so the *old* version of the optional larger database spkg will not be able to be applied to the *new* standard spkg. (Andrew, please correct me if I am wrong on this).
Anyway, the impression I got was that adding the additional curves published today would only take a few minutes. If longer, then go with what is on this ticket. But I don't know the right mechanism for changing the optional spkg.
comment:33 in reply to: ↑ 32 Changed 11 years ago by
Replying to cremona:
Replying to leif:
Replying to cremona:
This is for the default database (max conductor 10000); but this ticket cannot be merged until the large database is also ready to go and has been tested.
Which will be when?
I finished creating the data this morning, and Andrew Ohana is updating his optional spkg (?) now.
Working on it right now.
Can't we just merge this ticket and open a follow-up in case something goes wrong with the larger (anyway optional) database?
No, I don't think so, since the database has completely changed and so the *old* version of the optional larger database spkg will not be able to be applied to the *new* standard spkg. (Andrew, please correct me if I am wrong on this).
The optional spkgs mentioned the description work with the new database format, but there are a few places where the version of the package is hardcoded into the sage library (so holding off may be a good idea). I am going to remove these, so that maintaining the spkg is an easier chore.
comment:34 Changed 11 years ago by
- Description modified (diff)
Ok, I've updated the spkg, it now includes updating instructions in the spkg.txt as well. I've also added a patch that removes the hardcoded version numbers from the sage library, the instructions in the spkg are based off of this.
comment:35 follow-up: ↓ 36 Changed 11 years ago by
Andrew, is the optional spkg http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20110915.spkg with the larger database ready for "release"?
comment:36 in reply to: ↑ 35 Changed 11 years ago by
Replying to leif:
Andrew, is the optional spkg http://wstein.org/home/ohanar/cremona-database/database_cremona_ellcurve-20110915.spkg with the larger database ready for "release"?
It should be (the install script simply copies a single file)
comment:37 Changed 11 years ago by
- Merged in set to sage-4.7.2.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
comment:38 Changed 11 years ago by
- Description modified (diff)
Had to add a changelog entry for the previous version in Sage (0.1), since otherwise Jeroen's merger3 (meanwhile) rejects it (assuming it wasn't based on the previous one in Sage).
Corrected spkg at new location.
Changed 11 years ago by
"Proper" Mercurial changeset replacement patch. (Adds just the "User" field.)
comment:39 Changed 11 years ago by
- Description modified (diff)
I've attached one replacement patch which just adds the "# User ...
" line the original patch is lacking.
comment:40 Changed 11 years ago by
After installing the optionl daatbase on top of 4.7.2.alpha3 I had a doctest failure in devel/sage/doc/en/bordeaux_2008/elliptic_curves.rst . Nothing serious, just different gens for a curve (i.e. the ones in the database are not the same as those found, for a rank 2 curve).
comment:41 follow-ups: ↓ 42 ↓ 44 ↓ 54 Changed 11 years ago by
- Cc davidloeffler added
- Merged in sage-4.7.2.alpha3 deleted
- Resolution fixed deleted
- Status changed from closed to new
This is the cause of a serious speed regression. Message by Dave Loeffler:
I've found the culprit: this is a side-effect of #11587. If you look at trac_11587.patch, then it removes a couple of lines of code at lines 710-712 of sage/databases/cremona.py which sets the generators to [] if the gens aren't explicitly in the database but the rank is known to be 0. The new version doesn't perform this little check and so we waste loads of time doing pointless 2-descents.
comment:42 in reply to: ↑ 41 ; follow-up: ↓ 43 Changed 11 years ago by
Replying to jdemeyer:
This is the cause of a serious speed regression. Message by Dave Loeffler:
I've found the culprit: this is a side-effect of #11587. If you look at trac_11587.patch, then it removes a couple of lines of code at lines 710-712 of sage/databases/cremona.py which sets the generators to [] if the gens aren't explicitly in the database but the rank is known to be 0. The new version doesn't perform this little check and so we waste loads of time doing pointless 2-descents.
I was fearing something like that.
Why not open a follow-up (blocker) ticket for fixing that?
alpha3 is already out...
comment:43 in reply to: ↑ 42 Changed 11 years ago by
Replying to leif:
Why not open a follow-up (blocker) ticket for fixing that?
I thought it might be easier to change an existing ticket, especially if the change is small.
alpha3 is already out...
Doesn't really matter, I can re-merge the ticket in alpha4...
comment:44 in reply to: ↑ 41 Changed 11 years ago by
Replying to jdemeyer:
This is the cause of a serious speed regression. Message by Dave Loeffler:
I've found the culprit: this is a side-effect of #11587. If you look at trac_11587.patch, then it removes a couple of lines of code at lines 710-712 of sage/databases/cremona.py which sets the generators to [] if the gens aren't explicitly in the database but the rank is known to be 0. The new version doesn't perform this little check and so we waste loads of time doing pointless 2-descents.
It has always been rather a nuisance that the small default database differs from the large one not only in having fewer curves but in having less information about them (specifically, omits the known rank and generators). I don't want to delay this ticket longer than necessary (and apologise for the positive review which missed this glitch) but propose a follow-up which includes all known data for the small database too.
comment:45 Changed 11 years ago by
Follow-up or on this ticket will not make a difference because I plan to unmerge this ticket anyway if it causes such massive slowdown. So, do as you wish.
comment:46 follow-up: ↓ 47 Changed 11 years ago by
- Milestone changed from sage-4.7.2 to sage-4.7.3
This will be unmerged for the sage-4.7.2 release series. We really should get this working so that it can be merged again in sage-4.7.3.alpha0.
comment:47 in reply to: ↑ 46 Changed 11 years ago by
Replying to jdemeyer:
This will be unmerged for the sage-4.7.2 release series. We really should get this working so that it can be merged again in sage-4.7.3.alpha0.
Then we should also remove http://sagemath.org/packages/optional/database_cremona_ellcurve-20110915.spkg again, since it is in the new format (which only the alpha3 supports); either when alpha4 is officially released, or right before the final 4.7.2 gets out.
It shouldn't be hard to change its spkg-install
to just install the previous version (hard-coded URL, e.g. by recursively calling sage -i ...
or sage-spkg
) if the Sage version present doesn't support the new format, which is reasonable for the future anyway (i.e., when this ticket gets remerged). Cf. sage-release.
comment:48 Changed 11 years ago by
... or simply print a message instructing the user to instead do
$ ./sage -i http://sagemath.org/packages/archive/database_cremona_ellcurve-20071019.p0.spkg
(It should IMHO also still be in /packages/optional/
, along with its description, but I get a 404 if I replace archive
by optional
above.)
Ceterum censeo we should really split the spkg metadata (or Sage part) from its "real" contents (data or upstream tarballs), or at least additionally provide it in small files to be processed without the need to download the full spkgs in advance...
comment:49 follow-up: ↓ 50 Changed 11 years ago by
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.
comment:50 in reply to: ↑ 49 ; follow-up: ↓ 51 Changed 11 years ago by
Replying to cremona:
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).
comment:51 in reply to: ↑ 50 Changed 11 years ago by
- Description modified (diff)
Replying to leif:
Replying to cremona:
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.
comment:52 follow-up: ↓ 53 Changed 11 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:53 in reply to: ↑ 52 Changed 11 years ago by
Replying to davidloeffler:
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!
comment:54 in reply to: ↑ 41 ; follow-ups: ↓ 55 ↓ 59 Changed 11 years ago by
- Status changed from needs_review to positive_review
Everything works as expected.
@davidloeffler 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).
@cremona I've posted a new spkg with your expanded tables, same location as before.
comment:55 in reply to: ↑ 54 Changed 11 years ago by
Replying to ohanar:
Everything works as expected.
@davidloeffler 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).
@cremona 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.
comment:56 Changed 11 years ago by
- Merged in set to sage-4.7.3.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
comment:57 Changed 11 years ago by
- Description modified (diff)
comment:58 Changed 11 years ago by
- Description modified (diff)
I just updated the description to reflect the fact that the optional db now goes up to 200k.
comment:59 in reply to: ↑ 54 Changed 11 years ago by
- Description modified (diff)
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.
comment:61 Changed 11 years ago by
- Merged in changed from sage-4.7.3.alpha0 to sage-4.8.alpha0
- Milestone set to sage-4.8
comment:62 follow-up: ↓ 63 Changed 11 years ago by
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).
comment:63 in reply to: ↑ 62 ; follow-up: ↓ 64 Changed 11 years ago by
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).
comment:64 in reply to: ↑ 63 Changed 11 years ago by
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.
comment:65 follow-up: ↓ 66 Changed 11 years ago by
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?
Updated description to reflect the current status of the tables.