Opened 7 years ago
Closed 7 years ago
#18031 closed defect (fixed)
Bug in saturation for elliptic curves over Q
Reported by: | cremona | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-6.6 |
Component: | elliptic curves | Keywords: | saturation |
Cc: | Merged in: | ||
Authors: | John Cremona | Reviewers: | François Bissey |
Report Upstream: | Fixed upstream, in a later stable release. | Work issues: | |
Branch: | acefe94 (Commits, GitHub, GitLab) | Commit: | acefe94db857b8ae9de051203a5a61ff74df7d0c |
Dependencies: | Stopgaps: |
Description (last modified by )
A bug in eclib's saturation function is revealed here:
sage: E = EllipticCurve([0,-1,1,-266,968]) sage: Q1 = E([-1995,3674,125]) sage: Q2 = E([157,1950,1]) sage: E.saturation([Q1,Q2]) ([(-399/25 : 3674/125 : 1), (157 : 1950 : 1)], 1, 7.21429780216482) sage: P1, P2 = E.gens() sage: E.regulator() 0.801588644684980 sage: E.regulator_of_points([Q1,Q2]) 7.21429780216482
The points Q1,Q2 generate a subgroup of index 3 but the computed bound on the index is between 2 and 3. This will be fixed upstream.
Note that for this curve E.gens() gives correct generators but E.simon_two_descent() gives the above two points.
New upstream source at http://boxen.math.washington.edu/home/cremona/eclib-20150323.tar.bz2
Change History (10)
comment:1 Changed 7 years ago by
- Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.
comment:2 Changed 7 years ago by
- Description modified (diff)
comment:3 Changed 7 years ago by
- Report Upstream changed from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.
With upstream eclib-20150323:
sage: E = EllipticCurve([0,-1,1,-266,968]) sage: Q1 = E([-1995,3674,125]) sage: Q2 = E([157,1950,1]) sage: E.saturation([Q1,Q2]) ([(1 : -27 : 1), (157 : 1950 : 1)], 3, 0.801588644684981)
which will be put in as a doctest.
comment:4 Changed 7 years ago by
- Branch set to u/cremona/18031
comment:5 Changed 7 years ago by
- Branch changed from u/cremona/18031 to u/cremona/eclib-18031
- Commit set to acefe94db857b8ae9de051203a5a61ff74df7d0c
New commits:
acefe94 | update eclib to bugfix release 20150323 and add doctest
|
comment:6 Changed 7 years ago by
- Status changed from new to needs_review
comment:7 Changed 7 years ago by
- Reviewers set to François Bissey
- Status changed from needs_review to positive_review
lgtm
comment:8 Changed 7 years ago by
Thanks!
Release manager -- compared to 20150228 which was already merged, this only changes one source file, there are no API changes so this is a minor bug-fix release. Sorry it comes so soon after the previous one, but that's when I discovered the bug.
comment:9 Changed 7 years ago by
- Priority changed from major to blocker
I am making this a blocker, that'll get Volker's attention.
comment:10 Changed 7 years ago by
- Branch changed from u/cremona/eclib-18031 to acefe94db857b8ae9de051203a5a61ff74df7d0c
- Resolution set to fixed
- Status changed from positive_review to closed
Bug in eclib fixed. After testing, will update the upstream sources and the package information.