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

Some elliptic curve doctests fail when the optional database is installed #3793

Closed
JohnCremona opened this issue Aug 9, 2008 · 5 comments
Closed

Comments

@JohnCremona
Copy link
Member

A few of the doctests in ell_rational_field.py fail when the optional package database_cremona_ellcurve-20071019 is installed, mainly because for curves in the database the gens() as supplied by the database may differ from those computed on the fly. (In almost all cases the generators are not uniquely determined, being the generators of a finitely-generated abelian group. We have put some thought into how to make the generators canonical but have not yet succeeded.)

Component: algebraic geometry

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

@JohnCremona
Copy link
Member Author

comment:1

Attachment: 10109.patch.gz

To test this you should really test the doctests in ell_rational_field.py both before and after installing the database.

@sagetrac-mabshoff sagetrac-mabshoff mannequin added this to the sage-3.1 milestone Aug 10, 2008
@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Aug 10, 2008

comment:3

I ran the tests on 3.0.6 before and after installing the database, without applying the patch, and both tests passed everything. So... is this really necessary?

But I still think this looks good and should be applied, since it addresses some ambiguity that could be annoying.

@JohnCremona
Copy link
Member Author

comment:4

The point is that there was randomness in the old doctests: whenever they use E.gens() where E is an elliptic curve we cannot guarantee that the same gens are computed (on different systems, etc). As a special case of this ambiguity, the gens obtained from the database (which don't change! -- or at least ont very rarely, e.g. if they are found to be wrong) may not agree with computed gens.

I dealt with this by either inserting "# random", or by using explicit points instead of gens().

I hope that with this explanation you can give this (admittedly rather trivial) patch a positive review.

@JohnCremona
Copy link
Member Author

comment:5

ok, so it already has a positive review! Thanks!

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Aug 11, 2008

comment:6

Merged in Sage 3.1.alpha1

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Aug 11, 2008
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

2 participants