Ticket #11024 (needs_work enhancement)
Update Dokchitser calculator to compiled version
| Reported by: | mraum | Owned by: | craigcitro |
|---|---|---|---|
| Priority: | major | Milestone: | sage-5.10 |
| Component: | modular forms | Keywords: | |
| Cc: | cremona, rishi | Work issues: | |
| Report Upstream: | N/A | Reviewers: | John Cremona |
| Authors: | Martin Raum, Henri Cohen | Merged in: | |
| Dependencies: | Stopgaps: |
Description (last modified by mraum) (diff)
Cohen provided a C-version of Dokchitser's script. This wraps that script and cleans up the script. There were several issues present and doctests for the elliptic curves have been adapted to correctly set the maximal imaginary value.
Depends on:
Apply:
Attachments
-
trac-11024-dokchitser.patch
(43.3 KB) -
added by mraum 2 years ago.
-
dokchitser.spkg
(15.3 KB) -
added by mraum 2 years ago.
-
trac-11024-dokchitser-2.patch
(1.7 KB) -
added by mraum 2 years ago.
- Something went wrong with diff, this patch is better.
-
trac-11024-dokchitser-bordeaux.patch
(520 bytes) -
added by mraum 2 years ago.
Change History
comment:1 Changed 2 years ago by mraum
- Status changed from new to needs_review
- Description modified (diff)
comment:3 Changed 2 years ago by cremona
- Reviewers set to John Cremona
The patch and spkg installed OK (on 63-bit ubuntu) and I am now testing.
- The spkg needs a version number, so use the date (e.g. dokchitser-20110325.spkg)
- There needs to be an SPKG.txt file
- The spkg needs to be under mercurial version control (but *not* the src/ subdirectory).
- The spkg-install looks good!
These are trivialities, but necessary!
comment:5 Changed 2 years ago by cremona
- Status changed from needs_review to needs_work
I'll look at the new spkg. Meanwhile testing revealed one small problem:
sage -t "devel/sage-main/doc/en/bordeaux_2008/elliptic_curves.rst"
comment:6 Changed 2 years ago by cremona
New spkg: SPKG.txt seems to refer to a different spkg!!
Also I suggest adding .hgignore to list src/
comment:8 Changed 2 years ago by mraum
A tiny change to the Dokchitser wrapper. The old way of finding the library was kind of dump. Also the spkg now contains the .hgignore as you suggested.
Changed 2 years ago by mraum
-
attachment
trac-11024-dokchitser-2.patch
added
Something went wrong with diff, this patch is better.
comment:9 Changed 2 years ago by mraum
- Status changed from needs_work to needs_review
- Description modified (diff)
Fix the doctest in the Bordeaux notes. We need to set the maximal imaginary value correctly.
comment:10 Changed 2 years ago by jdemeyer
- Status changed from needs_review to needs_work
Martin: patches should be created using hg export tip instead of hg diff. If you use queues, be sure to set a meaningful commit message using hg qrefresh -e. The commit message should contain the ticket number on the first line.
comment:12 Changed 2 years ago by rishi
From my small tests, the patch works good, and it eliminated the bug which I saw yesterday. I would second Jeroen's request that there should be a meaningful commit message. Right now it is set of diffs.
I would have done it right now, but it will be hard for me to write an appropriate commit message without essentially reviewing it. I will do it next weekend if no one has done it by then.
comment:13 Changed 2 years ago by jdemeyer
If you are adding a new spkg to Sage, you should also update various files in SAGE_ROOT, in particular spkg/install and spkg/standard/deps.
comment:14 Changed 2 years ago by mraum
Jeroen added a comment to #11005, stating that it is probably better to add the single c-file to the Sage library. We probably also want to do this for the Dokchitser calculator.
I hope to have time for this soon.
