Ticket #715: trac_715-reviewer-take2.patch

File trac_715-reviewer-take2.patch, 19.3 KB (added by jpflori, 8 years ago)

  • sage/categories/action.pyx 

    # HG changeset patch
# User Jean-Pierre Flori <jean-pierre.flor@ssi.gouv.fr>
# Date 1334221874 -7200
# Node ID 3d76af023be692183fcaa41b18875402a36a2f75
# Parent  75769cbc372eb71c07067d54c3bf4c8e57cb2c04
#715: Reviewer patch

diff --git a/sage/categories/action.pyx b/sage/categories/action.pyx
    a b  
    11r"""
    2 Group, ring, etc. actions on objects. 
     2Group, ring, etc. actions on objects.
    33
    4 The terminology and notation used is suggestive of groups
    5 acting on sets, but this framework can be used for modules,
    6 algebras, etc.
     4The terminology and notation used is suggestive of groups acting on sets,
     5but this framework can be used for modules, algebras, etc.
    76
    8 A group action $G \times S \rightarrow S$ is a functor from $G$ to Sets. 
     7A group action $G \times S \rightarrow S$ is a functor from $G$ to Sets.
    98
    10 AUTHORS:
    11     -- Robert Bradshaw: initial version
     9.. WARNING::
     10
     11    An :class:`Action` object only keeps a weak reference to the underlying set
     12    which is acted upon. This decision was made in :trac:`715` in order to
     13    allow garbage collection within the coercion framework (this is where
     14    actions are mainly used) and avoid memory leaks.
     15
     16    ::
     17
     18        sage: from sage.categories.action import Action
     19        sage: class P: pass
     20        sage: A = Action(P(),P())
     21        sage: import gc
     22        sage: _ = gc.collect()
     23        sage: A
     24        Traceback (most recent call last):
     25        ...
     26        RuntimeError: This action acted on a set that became garbage collected
     27
     28    To avoid garbage collection of the underlying set, it is sufficient to
     29    create a strong reference to it before the action is created.
     30
     31    ::
     32
     33        sage: _ = gc.collect()
     34        sage: from sage.categories.action import Action
     35        sage: class P: pass
     36        sage: q = P()
     37        sage: A = Action(P(),q)
     38        sage: gc.collect()
     39        0
     40        sage: A
     41        Left action by <__main__.P instance at ...> on <__main__.P instance at ...>
     42
     43AUTHOR:
     44
     45- Robert Bradshaw: initial version
    1246"""
    1347
    1448#*****************************************************************************
    … …  
    130164            sage: A.left_domain() is R
    131165            True
    132166
    133         By trac ticket #715, there is only a weak reference to the underlying
    134         set. Hence, the underlying set may be garbage collected, even when the
     167        By :trac:`715`, there is only a weak reference to the underlying set.
     168        Hence, the underlying set may be garbage collected, even when the
    135169        action is still alive. This may result in a runtime error, as follows::
    136170
    137171            sage: from sage.categories.action import Action
    … …  
    143177            Left action by <__main__.P instance at ...> on <__main__.P instance at ...>
    144178            sage: del q
    145179            sage: import gc
    146             sage: n = gc.collect()
     180            sage: _ = gc.collect()
    147181            sage: A
    148182            Traceback (most recent call last):
    149183            ...

  • sage/matrix/action.pyx 

    diff --git a/sage/matrix/action.pyx b/sage/matrix/action.pyx
    a b  
    11"""
    2 These are the actions used by the coercion model for matrix and vector multiplications.
     2These are the actions used by the coercion model for matrix and vector
     3multiplications.
    34
    4 AUTHORS:
    5     -- Robert Bradshaw (2007-09): Initial version.
     5.. WARNING::
     6
     7    The class :class:`MatrixMulAction` and its descendants extends the class
     8    :class:`Action`. As a cosnequence objects from these classes only keep weak
     9    references to the underlying sets which are acted upon. This decision was
     10    made in :trac:`715` in order to allow garbage collection within the coercion
     11    framework, where actions are mainly used, and avoid memory leaks.
     12
     13    To ensure that the underlying set of such an object does not get garbage
     14    collected, it is sufficient to explicitely create a strong reference to it
     15    before creating the action.
     16
     17    ::
     18
     19        sage: MSQ = MatrixSpace(QQ, 2)
     20        sage: MSZ = MatrixSpace(ZZ['x'], 2)
     21        sage: A = MSQ.get_action(MSZ)
     22        sage: A
     23        Left action by Full MatrixSpace of 2 by 2 dense matrices over Rational Field on Full MatrixSpace of 2 by 2 dense matrices over Univariate Polynomial Ring in x over Integer Ring
     24        sage: import gc
     25        sage: _ = gc.collect()
     26        sage: A
     27        Left action by Full MatrixSpace of 2 by 2 dense matrices over Rational Field on Full MatrixSpace of 2 by 2 dense matrices over Univariate Polynomial Ring in x over Integer Ring
     28
     29.. NOTE::
     30
     31    The :func:`MatrixSpace` function caches the objects it creates. Therefore,
     32    the underlying set ``MSZ`` in the above example will not be garbage
     33    collected, even if it is not strongly ref'ed. Nonetheless, there is no
     34    guarantee that the set that is acted upon will always be cached in such a
     35    way, so that following the above example is good practice.
     36
     37AUTHOR:
     38
     39- Robert Bradshaw (2007-09): Initial version.
    640"""
    741
    842#*****************************************************************************
    … …  
    4074        """
    4175        EXAMPLES:
    4276
    43         By trac ticket #715, there only is a weak reference on the underlying set,
    44         so that it can be garbage collected if only the action itself is explicitly
    45         referred to. Hence, we first assign the involved matrix spaces to a
    46         variable::
     77        By :trac:`715`, there only is a weak reference on the underlying set,
     78        so that it can be garbage collected if only the action itself is
     79        explicitly referred to. Hence, we first assign the involved matrix
     80        spaces to a variable::
    4781
    4882            sage: MSQ = MatrixSpace(QQ, 2)
    4983            sage: MSZ = MatrixSpace(ZZ['x'], 2)
    … …  
    5589            Full MatrixSpace of 2 by 2 dense matrices over Univariate Polynomial Ring in x over Integer Ring
    5690            sage: A.codomain()
    5791            Full MatrixSpace of 2 by 2 dense matrices over Univariate Polynomial Ring in x over Rational Field
     92
     93        .. NOTE::
     94
     95            The :func:`MatrixSpace` function caches the object it creates.
     96            Therefore, the underlying set ``MSZ`` in the above example will not
     97            be garbage collected, even if it is not strongly ref'ed.
     98            Nonetheless, there is no guarantee that the set that is acted upon
     99            will always be cached in such a way, so that following the above
     100            example is good practice.
     101
    58102        """
    59103        return self.underlying_set()
    60104
    … …  
    64108        """
    65109        EXAMPLES:
    66110
    67         By trac ticket #715, there only is a weak reference on the underlying set,
    68         so that it can be garbage collected if only the action itself is explicitly
    69         referred to. Hence, we first assign the involved matrix spaces to a
    70         variable::
     111        By :trac:`715`, there only is a weak reference on the underlying set,
     112        so that it can be garbage collected if only the action itself is
     113        explicitly referred to. Hence, we first assign the involved matrix
     114        spaces to a variable::
    71115
    72116            sage: R.<x> = ZZ[]
    73117            sage: MSR = MatrixSpace(R, 3, 3)
    … …  
    81125            [  0   x]
    82126            [2*x 3*x]
    83127            [4*x 5*x]
     128
     129        .. NOTE::
     130
     131            The :func:`MatrixSpace` function caches the object it creates.
     132            Therefore, the underlying set ``MSZ`` in the above example will not
     133            be garbage collected, even if it is not strongly ref'ed.
     134            Nonetheless, there is no guarantee that the set that is acted upon
     135            will always be cached in such a way, so that following the above
     136            example is good practice.
     137
    84138        """
    85139        if not is_MatrixSpace(S):
    86140            raise TypeError, "Not a matrix space: %s" % S
    … …  
    90144        """
    91145        EXAMPLES:
    92146
    93         By trac ticket #715, there only is a weak reference on the underlying set,
    94         so that it can be garbage collected if only the action itself is explicitly
    95         referred to. Hence, we first assign the involved matrix spaces to a
    96         variable::
     147        By :trac:`715`, there only is a weak reference on the underlying set,
     148        so that it can be garbage collected if only the action itself is
     149        explicitly referred to. Hence, we first assign the involved matrix
     150        spaces to a variable::
    97151
    98152            sage: from sage.matrix.action import MatrixMatrixAction
    99153            sage: R.<x> = ZZ[]
    … …  
    103157            Left action by Full MatrixSpace of 3 by 3 dense matrices over Univariate Polynomial Ring in x over Integer Ring on Full MatrixSpace of 3 by 2 dense matrices over Rational Field
    104158            sage: A.codomain()
    105159            Full MatrixSpace of 3 by 2 dense matrices over Univariate Polynomial Ring in x over Rational Field
     160
     161        .. NOTE::
     162
     163            The :func:`MatrixSpace` function caches the object it creates.
     164            Therefore, the underlying set ``MSZ`` in the above example will not
     165            be garbage collected, even if it is not strongly ref'ed.
     166            Nonetheless, there is no guarantee that the set that is acted upon
     167            will always be cached in such a way, so that following the above
     168            example is good practice.
     169
    106170        """
    107171        if self.G.ncols() != self.underlying_set().nrows():
    108172            raise TypeError, "incompatible dimensions %s, %s" % (self.G.ncols(),  self.underlying_set().nrows())

  • sage/structure/coerce_dict.pyx 

    diff --git a/sage/structure/coerce_dict.pyx b/sage/structure/coerce_dict.pyx
    a b  
    2222
    2323cdef class TripleDictEraser:
    2424    """
    25     Erases items from a :class:`TripleDict` when a weak reference becomes invalid.
     25    Erases items from a :class:`TripleDict` when a weak reference becomes
     26    invalid.
    2627
    27     This is of internal use only. Instances of this class will be passed as a callback
    28     function when creating a weak reference.
     28    This is of internal use only. Instances of this class will be passed as a
     29    callback function when creating a weak reference.
    2930
    3031    EXAMPLES::
    3132
    … …  
    4647
    4748    AUTHOR:
    4849
    49     Simon King (2012-01)
     50    - Simon King (2012-01)
    5051    """
    5152
    5253    def __init__(self, D):
    … …  
    6465
    6566        """
    6667        self.D = D
     68
    6769    def __call__(self, r):
    6870        """
    6971        INPUT:
    7072
    7173        A weak reference with key.
    7274
    73         When this is called with a weak reference ``r``, then each item containing ``r``
    74         is removed from the associated :class:`TripleDict`. Normally, this only happens
    75         when a weak reference becomes invalid.
     75        When this is called with a weak reference ``r``, then each item
     76        containing ``r`` is removed from the associated :class:`TripleDict`.
     77        Normally, this only happens when a weak reference becomes invalid.
    7678
    7779        EXAMPLES::
    7880
    … …  
    9092            sage: n = gc.collect()
    9193            sage: len(T)    # indirect doctest
    9294            0
    93 
    9495        """
    9596        # r is a (weak) reference (typically to a parent),
    9697        # and it knows the stored key of the unique triple r() had been part of.
    … …  
    115116    """
    116117    This is a hashtable specifically designed for (read) speed in
    117118    the coercion model.
    118    
     119
    119120    It differs from a python dict in the following important ways:
    120    
     121
    121122       - All keys must be sequence of exactly three elements. All sequence
    122123         types (tuple, list, etc.) map to the same item.
    123124       - Comparison is done using the 'is' rather than '==' operator.
    124        
     125
    125126    There are special cdef set/get methods for faster access.
    126127    It is bare-bones in the sense that not all dictionary methods are
    127128    implemented.
    128    
     129
    129130    It is implemented as a list of lists (hereafter called buckets). The bucket
    130     is chosen according to a very simple hash based on the object pointer.
    131     and each bucket is of the form [id(k1), id(k2), id(k3), value, id(k1), id(k2),
    132     id(k3), value, ...], on which a linear search is performed.
    133    
     131    is chosen according to a very simple hash based on the object pointer,
     132    and each bucket is of the form [id(k1), id(k2), id(k3), value, id(k1),
     133    id(k2), id(k3), value, ...], on which a linear search is performed.
     134
    134135    To spread objects evenly, the size should ideally be a prime, and certainly
    135136    not divisible by 2.
    136    
    137    
     137
    138138    EXAMPLES::
    139    
     139
    140140        sage: from sage.structure.coerce_dict import TripleDict
    141141        sage: L = TripleDict(31)
    142142        sage: a = 'a'; b = 'b'; c = 'c'
    … …  
    183183        Traceback (most recent call last):
    184184        ...
    185185        KeyError: 'a'
    186        
     186
    187187    The following illustrates why even sizes are bad::
    188188
    189189        sage: L = TripleDict(4, L)
    … …  
    195195    Note that this kind of dictionary is also used for caching actions
    196196    and coerce maps. In previous versions of Sage, the cache was by
    197197    strong references and resulted in a memory leak in the following
    198     example. However, this leak was fixed by trac ticket #715, using
    199     weak references::
     198    example. However, this leak was fixed by trac ticket :trac:`715`,
     199    using weak references::
    200200
    201201        sage: K = GF(1<<55,'t')
    202202        sage: for i in range(50):
    … …  
    211211        sage: len(LE)    # indirect doctest
    212212        1
    213213
    214     AUTHOR::
     214    AUTHORS:
    215215
    216216    - Robert Bradshaw, 2007-08
     217
    217218    - Simon King, 2012-01
    218219    """
    219    
     220
    220221    def __init__(self, size, data=None, threshold=0):
    221222        """
    222         Create a special dict using triples for keys. 
    223        
     223        Create a special dict using triples for keys.
     224
    224225        EXAMPLES::
    225226
    226227            sage: from sage.structure.coerce_dict import TripleDict
    … …  
    238239        if data is not None:
    239240            for (k1,k2,k3), v in data.iteritems():
    240241                self.set(k1,k2,k3, v)
    241                
     242
    242243    def __len__(self):
    243244        """
    244245        The number of items in self.
    245        
     246
    246247        EXAMPLES::
    247248
    248249            sage: from sage.structure.coerce_dict import TripleDict
    … …  
    256257            3
    257258        """
    258259        return self._size
    259        
     260
    260261    def stats(self):
    261262        """
    262         The distribution of items in buckets. 
    263        
    264         OUTPUT::
     263        The distribution of items in buckets.
     264
     265        OUTPUT:
    265266
    266267        - (min, avg, max)
    267        
     268
    268269        EXAMPLES::
    269270
    270271            sage: from sage.structure.coerce_dict import TripleDict
    … …  
    277278            sage: for i in range(100): L[i,i,i] = None
    278279            sage: L.stats() # random
    279280            (0, 0.03325573661456601, 1)
    280            
     281
    281282            sage: L = TripleDict(1)
    282283            sage: for i in range(100): L[i,i,i] = None
    283284            sage: L.stats()
    … …  
    293294            else:
    294295                min = 0
    295296        return min, 1.0*size/len(self.buckets), max
    296        
     297
    297298    def bucket_lens(self):
    298299        """
    299         The distribution of items in buckets. 
    300        
     300        The distribution of items in buckets.
     301
    301302        OUTPUT:
    302        
    303         A list of how many items are in each bucket. 
    304        
     303
     304        A list of how many items are in each bucket.
     305
    305306        EXAMPLES::
    306307
    307308            sage: from sage.structure.coerce_dict import TripleDict
    … …  
    311312            [3, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 4, 3, 3, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 4, 3, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 4]
    312313            sage: sum(L.bucket_lens())
    313314            100
    314            
     315
    315316            sage: L = TripleDict(1)
    316317            sage: for i in range(100): L[i,i,i] = None
    317318            sage: L.bucket_lens()
    318319            [100]
    319320        """
    320321        return [len(self.buckets[i])/4 for i from 0 <= i < len(self.buckets)]
    321        
     322
    322323    def _get_buckets(self):
    323324        """
    324         The actual buckets of self, for debugging. 
    325        
    326         EXAMPLE::
     325        The actual buckets of self, for debugging.
     326
     327        EXAMPLES::
    327328
    328329            sage: from sage.structure.coerce_dict import TripleDict
    329330            sage: L = TripleDict(3)
    … …  
    332333            [[0, 0, 0, None], [], []]
    333334        """
    334335        return self.buckets
    335        
     336
    336337    def __getitem__(self, k):
    337338        """
    338339        EXAMPLES::
    … …  
    350351        except (TypeError,ValueError):
    351352            raise KeyError, k
    352353        return self.get(k1, k2, k3)
    353            
     354
    354355    cdef get(self, object k1, object k2, object k3):
    355356        cdef Py_ssize_t h = (<Py_ssize_t><void *>k1 + 13*<Py_ssize_t><void *>k2 ^ 503*<Py_ssize_t><void *>k3)
    356357        #if h < 0: h = -h  # shouldn't "%" result in a positive number anyway?
    … …  
    367368                    if <Py_ssize_t>tmp == <Py_ssize_t><void *>k3:
    368369                        return <object>PyList_GET_ITEM(bucket, i+3)
    369370        raise KeyError, (k1, k2, k3)
    370        
     371
    371372    def __setitem__(self, k, value):
    372373        """
    373374        EXAMPLES::
    … …  
    385386        except (TypeError,ValueError):
    386387            raise KeyError, k
    387388        self.set(k1, k2, k3, value)
    388            
     389
    389390    cdef set(self, object k1, object k2, object k3, value):
    390391        if self.threshold and self._size > len(self.buckets) * self.threshold:
    391392            self.resize()
    392393        cdef Py_ssize_t h1 = <Py_ssize_t><void *>k1
    393394        cdef Py_ssize_t h2 = <Py_ssize_t><void *>k2
    394395        cdef Py_ssize_t h3 = <Py_ssize_t><void *>k3
    395        
     396
    396397        cdef Py_ssize_t h = (h1 + 13*h2 ^ 503*h3)
    397398        #if h < 0: h = -h   # shouldn't "%" return a positive number anyway?
    398399        cdef Py_ssize_t i
    … …  
    429430            except TypeError:
    430431                PyList_Append(_refcache.setdefault((h1, h2, h3), []),k3)
    431432        self._size += 1
    432            
     433
    433434    def __delitem__(self, k):
    434435        """
    435436        EXAMPLES::
    … …  
    459460                self._size -= 1
    460461                return
    461462        raise KeyError, k
    462        
     463
    463464    def resize(self, int buckets=0):
    464465        """
    465         Changes the number of buckets of self, while preserving the contents. 
    466        
    467         If the number of buckets is 0 or not given, it resizes self to the 
    468         smallest prime that is at least twice as large as self. 
    469        
     466        Changes the number of buckets of self, while preserving the contents.
     467
     468        If the number of buckets is 0 or not given, it resizes self to the
     469        smallest prime that is at least twice as large as self.
     470
    470471        EXAMPLES::
    471472
    472473            sage: from sage.structure.coerce_dict import TripleDict
    … …  
    508509            [((1, 2, 3), None)]
    509510        """
    510511        return TripleDictIter(self)
    511        
     512
    512513    def __reduce__(self):
    513514        """
    514         Note that we don't expect equality as this class concerns itself with 
    515         object identity rather than object equality. 
     515        Note that we don't expect equality as this class concerns itself with
     516        object identity rather than object equality.
    516517
    517518        EXAMPLES::
    518519
    … …  
    525526            [((1, 2, 3), True)]
    526527        """
    527528        return TripleDict, (len(self.buckets), dict(self.iteritems()), self.threshold)
    528        
    529529
    530530cdef class TripleDictIter:
    531531    def __init__(self, pairs):
    … …  
    540540        """
    541541        self.pairs = pairs
    542542        self.buckets = iter(self.pairs.buckets)
     543
    543544    def __iter__(self):
    544545        """
    545546        EXAMPLES::
    … …  
    551552            ((1, 2, 3), None)
    552553        """
    553554        return self
     555
    554556    def __next__(self):
    555557        """
    556558        EXAMPLES::
    … …  
    578580            return self.next()
    579581
    580582
    581 
    582583cdef long next_odd_prime(long n):
    583584    if n % 2 == 0:
    584585        n -= 1