Trac #15692: Value of cached methods should not always be pickled

In #15278, the hash of graphs was turned into a `@cached_method`. If a
cached method is pickled, then the cached value is pickled as well. In
the case of a hash value, this is not always what we want, since the
hash value might depend on the memory address of an object, or worse:
The hash function might change in future implementations.

That's to say: If you save object O now, together with hash value 123,
and later load it, then hash(O) will still be 123, even if a newly
constructed object P with O==P might have hash(P)==456, because of a
change in the hash function.

This obviously is a problem. There is something called
`sage.misc.cachefunc.ClearCacheOnPickle`, but I don't know if this would
really help in this case. Also, this would clear ''all'' cache values at
once. Also, I don't know if it works for cached methods that do not take
arguments (beside `self`).

I suggest that we should instead have an optional parameter for
`sage.misc.cachefunc.CachedMethodCallerNoArgs` that determines whether
or not the cached value is preserved, and use it on graphs (and future
applications of cached method on `__hash__`).

URL: https://trac.sagemath.org/15692
Reported by: SimonKing
Ticket author(s): Julian Rueth
Reviewer(s): Vincent Delecroix, David Roe

src/sage/combinat/root_system/ambient_space.py

@@ -13,11 +13,9 @@
 from sage.combinat.free_module import CombinatorialFreeModule, CombinatorialFreeModuleElement
 from .weight_lattice_realizations import WeightLatticeRealizations
 from sage.rings.all import ZZ, QQ
-from sage.misc.cachefunc import ClearCacheOnPickle
-from sage.modules.free_module_element import vector
 from sage.categories.homset import End
 
-class AmbientSpace(ClearCacheOnPickle, CombinatorialFreeModule):
+class AmbientSpace(CombinatorialFreeModule):
     r"""
     Abstract class for ambient spaces
 

src/sage/combinat/root_system/root_space.py

@@ -10,17 +10,14 @@
 from __future__ import print_function
 from __future__ import absolute_import
 
-from sage.misc.cachefunc import ClearCacheOnPickle, cached_method, cached_in_parent_method
+from sage.misc.cachefunc import cached_method, cached_in_parent_method
 from sage.rings.all import ZZ
 from sage.combinat.free_module import CombinatorialFreeModule, CombinatorialFreeModuleElement
 from .root_lattice_realizations import RootLatticeRealizations
 from sage.misc.cachefunc import cached_in_parent_method
 import functools
 
-# TODO: inheriting from ClearCacheOnPickle is a technical detail unrelated to root spaces
-# could we abstract this somewhere higher?
-
-class RootSpace(ClearCacheOnPickle, CombinatorialFreeModule):
+class RootSpace(CombinatorialFreeModule):
     r"""
     The root space of a root system over a given base ring
 

src/sage/combinat/root_system/weyl_group.py

@@ -41,7 +41,7 @@
 from sage.groups.matrix_gps.group_element import MatrixGroupElement_gap
 from sage.rings.all import ZZ, QQ
 from sage.interfaces.gap import gap
-from sage.misc.cachefunc import cached_method, ClearCacheOnPickle
+from sage.misc.cachefunc import cached_method
 from sage.combinat.root_system.cartan_type import CartanType
 from sage.combinat.root_system.cartan_matrix import CartanMatrix
 from sage.matrix.constructor import matrix, diagonal_matrix
@@ -196,7 +196,7 @@ def WeylGroup(x, prefix=None):
     return WeylGroup_gens(ct.root_system().root_space(), prefix=prefix)
 
 
-class WeylGroup_gens(ClearCacheOnPickle, UniqueRepresentation,
+class WeylGroup_gens(UniqueRepresentation,
                      FinitelyGeneratedMatrixGroup_gap):
 
     @staticmethod

src/sage/misc/cachefunc.pxd

@@ -15,14 +15,18 @@ cdef class CachedFunction(object):
     cdef fix_args_kwds(self, tuple args, dict kwds)
     cdef empty_key
     cdef key
+    cdef bint do_pickle
 
 cdef class CachedMethod(object):
     cdef str _cache_name
     cdef public str __name__
     cdef public str __module__
     cdef CachedFunction _cachedfunc
     cdef Py_ssize_t nargs
-    cpdef dict _get_instance_cache(self, inst)
+    cpdef _get_instance_cache(self, inst)
+
+cdef class CacheDict(dict):
+    pass
 
 cdef class CachedInParentMethod(CachedMethod):
     pass