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

Use cached_method for FractionField.is_exact #12527

Closed
nthiery opened this issue Feb 17, 2012 · 4 comments
Closed

Use cached_method for FractionField.is_exact #12527

nthiery opened this issue Feb 17, 2012 · 4 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented Feb 17, 2012

Before the patch:

    sage: sage: P = ZZ['x']
    sage: R = P.fraction_field()
    sage: %timeit R.is_exact()
    625 loops, best of 3: 3.7 µs per loop

After the patch:

    sage: R=FractionField(ZZ['x'])
    sage: %timeit R.is_exact()
    625 loops, best of 3: 485 ns per loop

Thank you Simon King for #11115!

This is non negligible because is_exact is called basically each
time an element is constructed or arithmetic is performed. Here we
have a 10% improvement on constructing an element:

Before the patch:

    sage: x = P.gen()
    sage: y = x+1
    sage: %timeit R(x,y,coerce=False)
    625 loops, best of 3: 17.7 µs per loop

After the patch:

    sage: x = P.gen()
    sage: y = x+1
    sage: %timeit R(x,y,coerce=False)
    625 loops, best of 3: 16 µs per loop

CC: @simon-king-jena

Component: performance

Author: Nicolas M. Thiéry

Reviewer: David Loeffler

Merged: sage-5.0.beta9

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

@nthiery nthiery added this to the sage-5.0 milestone Feb 17, 2012
@nthiery
Copy link
Contributor Author

nthiery commented Feb 17, 2012

comment:1

Attachment: trac_12527-fraction_field-is_exact_optimization-nt.patch.gz

Simon: you deserve this trivial review, for all your hard work on other related tickets :-)

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 12, 2012

comment:2

Isn't patchbot great? It makes reviewing patches like this a 10-second job. Sorry Simon!

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 12, 2012

Changed reviewer from Simon King to David Loeffler

@jdemeyer
Copy link

Merged: sage-5.0.beta9

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