Opened 5 years ago
Closed 4 years ago
#20737 closed defect (fixed)
Use of representative_prime may fail in finding semi-global minimal models of elliptic curves
Reported by: | cremona | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.5 |
Component: | elliptic curves | Keywords: | global minimal model |
Cc: | Merged in: | ||
Authors: | John Cremona | Reviewers: | Chris Wuthrich |
Report Upstream: | N/A | Work issues: | |
Branch: | 0d771e0 (Commits, GitHub, GitLab) | Commit: | 0d771e0dbdf553fa17e2f6b432acf349ada80d8d |
Dependencies: | Stopgaps: |
Description
When an elliptic curve E defined over a number field has no global minimal model, the obstruction is a non-trivial ideal class and E.global_minimal_model(semi_global=True) finds a prime ideal in that class and returns a model minimal at all primes but that one. However the line in kraus.py (line 916) does that using {{{P = c.representative_prime()}} which can fail since by default only primes of norm up to 1000 are tested, as that is the default value of the parameter norm_bound of representative_prime().
Example (an elliptic curve with everywhere good reduction over an imaginary quadratic field of class number 3315):
sage: K.<a> = NumberField(x^2-x+31821453) sage: ainvs = (0, 0, 0, -382586771000351226384*a - 2498023791133552294513515, 358777608829102441023422458989 744*a + 1110881475104109582383304709231832166) sage: E = EllipticCurve(ainvs) sage: assert E.conductor().norm() == 1 sage: E.global_minimal_model(semi_global=True) <boom> RuntimeError: No prime of norm less than 1000 found in class Fractional ideal class (1569, a + 867)
Experiment with this example shows that the smallest prime in that class has norm 23567.
The obvious solution is to keep doubling the bound until success. The ideal in the error message has norm 1569 and is a product of two primes so could be used instead but then the model returned would not be "semi-global" in the above sense.
Whatever is decided, the semi-global=True option (which is not the default) should not cause a run-time error.
Change History (4)
comment:1 Changed 5 years ago by
- Branch set to u/cremona/20737
- Commit set to 0d771e0dbdf553fa17e2f6b432acf349ada80d8d
- Milestone changed from sage-7.3 to sage-7.5
- Status changed from new to needs_review
comment:2 Changed 5 years ago by
There are currently 2 patchbot report for this ticket: one says that all tests pass but that some plugin gailed (what's that?). the other says that a test failed but this is bogus, just some file being tested timed out for reasons nothing to do with this ticket.
But I am guessing that the negative reports will prevent a uman with some intelligence looking at this. Not good!
comment:3 Changed 5 years ago by
- Reviewers set to Chris Wuthrich
- Status changed from needs_review to positive_review
All fine. Tests pass and even the patchbot is fully happy.
The new test is a bit long, but I can't see a way to make that faster and I agree that a test for this should be added.
comment:4 Changed 4 years ago by
- Branch changed from u/cremona/20737 to 0d771e0dbdf553fa17e2f6b432acf349ada80d8d
- Resolution set to fixed
- Status changed from positive_review to closed
This fixes the problem by calling the representative_prime function with an explicit bound and repeatedly increaisng the bound until success. It might be preferable to rewrite the representative_prime method on ideal classes to allow (in effect) bound=Infinity, but that can be done elsewhere.
I added a new test with the example given here, but it takes 15s to find the prime so the test is tagged with # long time.
New commits:
#20737: fix bug causing failure in semi-global minimal models of elliptic curves