#11115: Rewrite cached_method in Cython
-------------------------------------------------------------------+--------
   Reporter:  SimonKing                                            |          
Owner:  jason                           
       Type:  enhancement                                          |         
Status:  needs_review                    
   Priority:  major                                                |      
Milestone:  sage-pending                    
  Component:  misc                                                 |       
Keywords:  category cython cache           
Work_issues:  docbuild                                             |       
Upstream:  N/A                             
   Reviewer:  Nicolas M. ThiƩry, Andrey Novoseltsev, Volker Braun  |         
Author:  Simon King                      
     Merged:                                                       |   
Dependencies:  #9976 #11298 #11342 #9138 #11815
-------------------------------------------------------------------+--------
Changes (by SimonKing):

  * status:  needs_work => needs_review


Old description:

> Broadening the original description of the ticket:
>
> The aim is to provide a Cython version of the cached_method decorator.
> There are three benefits:
>
> 1)
> Speed (see timings in the comments)
>
> 2)
> Using cached methods in Cython code.
>
> 3) Parent and element methods of categories
>
> Let me elaborate on the last point:
>
> In order to make full use of the parent and element methods of a
> category, it should be possible to define a ''cached'' method in the
> category, so that any object of that category inherits it ''with
> caching''.
>
> Currently, it fails as follows:
> {{{
> sage: class MyCategory(Category):
> ....:     def super_categories(self):
> ....:         return [Objects()]
> ....:     class ParentMethods:
> ....:         @cached_method
> ....:         def f(self, x):
> ....:             return x^2
> ....:
> sage: cython("""from sage.structure.parent cimport Parent
> ....: cdef class MyClass(Parent): pass
> ....: """)
> sage: O = MyClass(category=MyCategory())
> sage: O.f(4)
> 16
> sage: O.f(x) is O.f(x)
> False
> }}}
>
> So, the method is inherited, but not cached.
>

> '''Apply:'''
>  1. [attachment:trac11115-cached_cython.patch]
>  1. [attachment:trac11115_element_with_cache.patch]
>  1. [attachment:trac11115_cached_function_pickling.patch]
>  1. [attachment:trac_11115_reviewer.patch]
>
> Note that, if you want to '''remove''' the patches after testing them,
> you need to do
> {{{
> rm $SAGE_ROOT/devel/sage/build/sage/misc/cachefunc.*
> rm $SAGE_ROOT/devel/sage/build/*/sage/misc/cachefunc.*
> }}}
> Otherwise, `sage -br` would not work.

New description:

 Broadening the original description of the ticket:

 The aim is to provide a Cython version of the cached_method decorator.
 There are three benefits:

 1)
 Speed (see timings in the comments)

 2)
 Using cached methods in Cython code.

 3) Parent and element methods of categories

 Let me elaborate on the last point:

 In order to make full use of the parent and element methods of a category,
 it should be possible to define a ''cached'' method in the category, so
 that any object of that category inherits it ''with caching''.

 Currently, it fails as follows:
 {{{
 sage: class MyCategory(Category):
 ....:     def super_categories(self):
 ....:         return [Objects()]
 ....:     class ParentMethods:
 ....:         @cached_method
 ....:         def f(self, x):
 ....:             return x^2
 ....:
 sage: cython("""from sage.structure.parent cimport Parent
 ....: cdef class MyClass(Parent): pass
 ....: """)
 sage: O = MyClass(category=MyCategory())
 sage: O.f(4)
 16
 sage: O.f(x) is O.f(x)
 False
 }}}

 So, the method is inherited, but not cached.


 '''Apply:'''
  1. [attachment:trac11115-cached_cython.patch]
  1. [attachment:trac11115_element_with_cache.patch]
  1. [attachment:trac11115_cached_function_pickling.patch]
  1. [attachment:trac_11115_reviewer.patch]
  1. [attachment:trac_11115_docfix.patch]

 Note that, if you want to '''remove''' the patches after testing them, you
 need to do
 {{{
 rm $SAGE_ROOT/devel/sage/build/sage/misc/cachefunc.*
 rm $SAGE_ROOT/devel/sage/build/*/sage/misc/cachefunc.*
 }}}
 Otherwise, `sage -br` would not work.

--

Comment:

 Hooray! With the blank line, the missing first lines of the
 `modp_splitting_data` doc string (and other instances) re-appear, and most
 importantly the error has vanished:
 {{{
 king@mpc622:/mnt/local/king/SAGE/rebase/sage-4.7.2.alpha4$ grep -i error
 dochtml.log
 reading sources... [ 79%] sage/rings/padics/precision_error
 writing output... [ 79%] sage/rings/padics/precision_error
 }}}

 The new patch does the following:

  * Fix some wrong docstring formats that I stumbled over.
  * Add the required blank line after the embedding information in the doc
 strings of cached methods.
  * In two cases, a cached function has been created by first defining a
 non-cached function, e.g. `adem_`, and then defining `adem =
 CachedFunction(adem_)`. If I am not mistaken, ''both'' functions would be
 included in the docs, which is not good. So, for aesthetic reasons, I
 rewrote it using `@cached_function`.

 I am now running doc tests. It could be that some tests in sage_inspect
 will fail. But I am confident enough to turn it into "needs review".

 For the patch bot:

 Apply trac11115-cached_cython.patch trac11115_element_with_cache.patch
 trac11115_cached_function_pickling.patch trac_11115_reviewer.patch
 trac_11115_docfix.patch

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11115#comment:152>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/sage-trac?hl=en.

Reply via email to