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

Improve caching in EllipticCurve_generic #16129

Closed
saraedum opened this issue Apr 11, 2014 · 21 comments
Closed

Improve caching in EllipticCurve_generic #16129

saraedum opened this issue Apr 11, 2014 · 21 comments

Comments

@saraedum
Copy link
Member

The way caching is implemented in EllipticCurve_generic causes some trouble
at #11895. Several methods, such as _multiple_x_numerator,
_multiple_x_denominator, division_polynomial_0 should be rewritten such that they do not use a cache parameter anymore.

CC: @JohnCremona @defeo @sagetrac-sbesnier

Component: elliptic curves

Author: Julian Rüth

Branch/Commit: 4c00ee1

Reviewer: Peter Bruin

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

@saraedum saraedum added this to the sage-6.2 milestone Apr 11, 2014
@saraedum
Copy link
Member Author

Branch: u/saraedum/ticket/16129

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2014

Commit: b9b79ca

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

b9b79caUse @cached_method decorator for _multiple_x_numerator/denominator in EllipticCurve_generic

@saraedum

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2014

Changed commit from b9b79ca to e12fc10

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

e12fc10Use cached_function to cache computations in division_polynomial_0 of elliptic curves

@saraedum
Copy link
Member Author

Author: Julian Rüth

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

72c46daFixed a typo in a reference.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 11, 2014

Changed commit from e12fc10 to 72c46da

@pjbruin
Copy link
Contributor

pjbruin commented Apr 23, 2014

Work Issues: resolve merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2014

Changed commit from 72c46da to fe8b5a9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

fe8b5a9Merge branch 'develop' into ticket/16129

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2014

Changed commit from fe8b5a9 to afdfca4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

afdfca4Merge branch 'develop' into ticket/16129

@saraedum
Copy link
Member Author

Changed work issues from resolve merge conflict to none

@pjbruin
Copy link
Contributor

pjbruin commented Apr 28, 2014

comment:14

Looks good to me; now running doctests.

I'm just slightly worried that some of the methods may cache too much, because the caching key depends on x, which can just be a concrete x-value rather than a polynomial variable. However, this already seems to have been the case before this ticket, so maybe we can keep it like that for now.

@pjbruin
Copy link
Contributor

pjbruin commented Apr 28, 2014

Reviewer: Peter Bruin

@pjbruin
Copy link
Contributor

pjbruin commented Apr 28, 2014

Changed branch from u/saraedum/ticket/16129 to u/pbruin/16129-EllipticCurve_caching

@pjbruin
Copy link
Contributor

pjbruin commented Apr 28, 2014

comment:15

All doctests pass, but building documentation was interrupted due to a warning which was caused by missing indentation. Hence a trivial reviewer patch.

@pjbruin
Copy link
Contributor

pjbruin commented Apr 28, 2014

Changed commit from afdfca4 to 4c00ee1

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

vbraun commented May 7, 2014

Changed branch from u/pbruin/16129-EllipticCurve_caching to 4c00ee1

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

3 participants