#12601: @cached_method does not work for special methods
-------------------------------------------------+-------------------------
Reporter: saraedum | Owner: jason
Type: enhancement | Status:
Priority: minor | needs_review
Component: misc | Milestone: sage-5.12
Keywords: cached_method, cache, | Resolution:
operator, special method | Merged in:
Authors: Simon King | Reviewers:
Report Upstream: Completely fixed; Fix | Work issues:
reported upstream | Commit:
Branch: | Stopgaps:
Dependencies: #15038 |
-------------------------------------------------+-------------------------
Changes (by {'newvalue': u'Simon King', 'oldvalue': ''}):
* status: new => needs_review
* dependencies: => #15038
* author: => Simon King
* upstream: Reported upstream. No feedback yet. => Completely fixed; Fix
reported upstream
Comment:
In principle, it would be possible to change `CachedMethod.__get__` to
deal both with usual and special methods. However, the price to pay would
be either a slow-down when using cached methods on instances that don't
allow attribute assignment, or cached special methods would be slow.
Therefore, I created a new class `CachedSpecialMethod` that overloads the
`__get__` method. Now, cached_method is a function (not an alias of
`CachedMethod`) that chooses amongst `CachedMethod` and
`CachedSpecialMethod` based on the name of the wrapped method.
I also tried to tweak the case of (usual) cached methods in the case of
forbidden attribute assignment.
'''__Timings and examples__'''
__With the patch:__
{{{
sage: class CCache(object):
....: @cached_method
....: def __hash__(self):
....: print "computing hash"
....: return int(5)
....: @cached_method
....: def f(self):
....: print "computing cached method with no args"
....: return 4
....: @cached_method
....: def g(self, x):
....: print "computing cached method with args"
....: return x*2
....:
sage: class CNoncache(object):
....: def __hash__(self):
....: return int(5)
....: def f(self):
....: return 4
....: def g(self, x):
....: return x*2
....:
sage: c = CCache()
sage: hash(c)
computing hash
5
sage: hash(c)
5
sage: c.f()
computing cached method with no args
4
sage: c.f()
4
sage: c.g(4)
computing cached method with args
8
sage: c.g(4)
8
sage: c.g(int(4))
8
sage: %timeit hash(c)
1000000 loops, best of 3: 241 ns per loop
sage: %timeit c.f()
10000000 loops, best of 3: 139 ns per loop
sage: %timeit c.g(4)
1000000 loops, best of 3: 694 ns per loop
sage: n = CNoncache()
sage: %timeit hash(n)
1000000 loops, best of 3: 827 ns per loop
sage: %timeit n.f()
1000000 loops, best of 3: 389 ns per loop
sage: %timeit n.g(4)
1000000 loops, best of 3: 669 ns per loop
}}}
Note that the cached hash is ''faster'' than an uncached hash that returns
a constant number!!
Without attribute assignment (special methods can't be wrapped in Cython,
so I can't wrap `__hash__`):
{{{
sage: cython("""
....: from sage.structure.parent cimport Parent
....: from sage.misc.cachefunc import cached_method
....: cdef class MyParent(Parent):
....: @cached_method
....: def f(self):
....: return 4
....: @cached_method
....: def g(self,x):
....: return x*2
....: """)
sage: m = MyParent()
sage: m.f() is m.f()
True
sage: m.g(4) is m.g(4)
True
sage: %timeit m.f()
1000000 loops, best of 3: 237 ns per loop
sage: %timeit m.g(4)
1000000 loops, best of 3: 831 ns per loop
}}}
__Without the patch__
The classes and instances are defined as above. As we know, caching the
hash did not work, but of course it did work on usual methods:
{{{
sage: hash(c)
computing hash
5
sage: hash(c)
computing hash
5
sage: c.f() is c.f()
computing cached method with no args
True
sage: c.g(int(4)) is c.g(4)
computing cached method with args
True
}}}
The timing with the possibility to assign attributes is slightly better
with my patch (but I don't know how stable this is):
{{{
sage: %timeit c.f()
10000000 loops, best of 3: 137 ns per loop
sage: %timeit c.g(4)
1000000 loops, best of 3: 702 ns per loop
}}}
The timings without attribute assignment did improve with my patch as
well:
{{{
sage: %timeit m.f()
1000000 loops, best of 3: 334 ns per loop
sage: %timeit m.g(4)
1000000 loops, best of 3: 958 ns per loop
}}}
'''__Conclusion__'''
I believe that my patch solves the problem and would be glad about a
review :)
I didn't run the full test suite yet.
It could be that there is an interference with #15038, which I thus add as
a dependency (it already has positive review).
--
Ticket URL: <http://trac.sagemath.org/ticket/12601#comment:9>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
