Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug in elliptic curve two_descent command #10665

Closed
williamstein opened this issue Jan 20, 2011 · 11 comments
Closed

bug in elliptic curve two_descent command #10665

williamstein opened this issue Jan 20, 2011 · 11 comments

Comments

@williamstein
Copy link
Contributor

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: 

Component: elliptic curves

Author: Peter Bruin

Branch/Commit: b437f35

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/10665

@sagetrac-weigandt
Copy link
Mannequin

sagetrac-weigandt mannequin commented Mar 23, 2011

comment:1

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."

@JohnCremona
Copy link
Member

comment:2

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.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@chriswuthrich
Copy link
Contributor

comment:5

Even after #10108, this is still a problem !

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@pjbruin
Copy link
Contributor

pjbruin commented Jun 23, 2014

comment:7

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.

@pjbruin
Copy link
Contributor

pjbruin commented Jun 23, 2014

@pjbruin
Copy link
Contributor

pjbruin commented Jun 23, 2014

Author: Peter Bruin

@pjbruin
Copy link
Contributor

pjbruin commented Jun 23, 2014

comment:8

(Note: the change to if not self.__descent.ok() is just a trivial simplification, the important line is self.__saturate = -2.)

@pjbruin
Copy link
Contributor

pjbruin commented Jun 23, 2014

Commit: b437f35

@fchapoton
Copy link
Contributor

comment:9

ok, the patchbot is green, and the patch looks simple enough to me.

So let me give a positive review.

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@vbraun
Copy link
Member

vbraun commented Jun 26, 2014

Changed branch from u/pbruin/10665-mwrank_elliptic_curve_two_descent to b437f35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants