Opened 6 years ago
Closed 3 years ago
#10665 closed defect (fixed)
bug in elliptic curve two_descent command
Reported by: | was | Owned by: | cremona |
---|---|---|---|
Priority: | minor | Milestone: | sage-6.3 |
Component: | elliptic curves | Keywords: | |
Cc: | Merged in: | ||
Authors: | Peter Bruin | Reviewers: | Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | b437f35 (Commits) | Commit: | b437f3539dbda9bdb16c400d406925f3f2a20293 |
Dependencies: | Stopgaps: |
Description
Don't do it twice:
sage: E = EllipticCurve([1,1,0,0,528]) sage: E.two_descent(verbose=False) True sage: E.two_descent(verbose=False) --------------------------------------------------------------------------- RuntimeError Traceback (most recent call last) /Users/wstein/<ipython console> in <module>() /Users/wstein/sage/install/sage-4.6.1/local/lib/python2.6/site-packages/sage/schemes/elliptic_curves/ell_rational_field.pyc in two_descent(self, verbose, selmer_only, first_limit, second_limit, n_aux, second_descent) 868 n_aux, second_descent) 869 if C.certain(): --> 870 self.__gens[True] = [self.point(x, check=True) for x in C.gens()] 871 self.__gens[True].sort() 872 self.__rank[True] = len(self.__gens[True]) /Users/wstein/sage/install/sage-4.6.1/local/lib/python2.6/site-packages/sage/libs/mwrank/interface.pyc in gens(self) 598 from sage.misc.all import preparse 599 from sage.rings.all import Integer --> 600 return eval(preparse(self.__two_descent_data().getbasis().replace(":",","))) 601 602 def certain(self): RuntimeError:
Change History (10)
comment:1 Changed 6 years ago by
comment:2 Changed 6 years ago by
And there are patches at #10108 which represent a lot of work which have been lying about unused and unmerged just because they are not perfect, even if Sage is better with them than without.
comment:3 Changed 4 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:4 Changed 3 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:5 Changed 3 years ago by
Even after #10108, this is still a problem !
comment:6 Changed 3 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:7 Changed 3 years ago by
The cause is that each time you call mwrank_EllipticCurve.two_descent()
, it creates a new __descent
(the 2-descent data) but does not reset the __saturate
bound. Therefore no saturation is done on the new __descent
data, which is bad because getbasis()
depends on the saturation having been done. I'm now testing a patch.
comment:8 Changed 3 years ago by
- Branch set to u/pbruin/10665-mwrank_elliptic_curve_two_descent
- Commit set to b437f3539dbda9bdb16c400d406925f3f2a20293
- Status changed from new to needs_review
(Note: the change to if not self.__descent.ok()
is just a trivial simplification, the important line is self.__saturate = -2
.)
comment:9 Changed 3 years ago by
- Reviewers set to Frédéric Chapoton
- Status changed from needs_review to positive_review
ok, the patchbot is green, and the patch looks simple enough to me.
So let me give a positive review.
comment:10 Changed 3 years ago by
- Branch changed from u/pbruin/10665-mwrank_elliptic_curve_two_descent to b437f3539dbda9bdb16c400d406925f3f2a20293
- Resolution set to fixed
- Status changed from positive_review to closed
This is probably explained by Cremona's comment at #10108:
"I think I can explain this. The problem to be solved for this ticket was that if mwrank is given incomplete but otherwise correct input, it just waits for the rest of the input, making Sage appear to hang. To fix this I made sure that the input provided by Sage always has some other stuff appended to it, so mwrank never has insufficient input. But then, the *next* time input is sent to mwrank, there is likely to be still some of that extra stuff in its input buffer. To get around that (I thought) I made sure that mwrank was restarted at every call.
Clearly what I did was insufficient, but this does explain who the order of executing commands does matter."