fix underlying bug behind DensityDist stackoverflow

tl:dr: The hashable helper function did not appropiately deal with tuples (and the test case did not actually test the memoization).

In the process of prior-predictive sampling a model involving a DensityDist, the _compile_theano_function function was called with arguments (sd__log_, []).
The _compile_theano_function has a pm.memo.memoize-decorator, which relies on the pm.memo.hashable for hashing of typically unhashable objects.
The "hashable" function incorrectly handled tuples, eventually causing a stackoverflow error on Windows.

Author: michaelosthege

pymc3/memoize.py

@@ -13,7 +13,7 @@
 #   limitations under the License.
 
 import functools
-import pickle
+import dill
 import collections
 from .util import biwrap
 
@@ -23,7 +23,16 @@
 @biwrap
 def memoize(obj, bound=False):
     """
-    An expensive memoizer that works with unhashables
+    Decorator to apply memoization to expensive functions.
+    It uses a custom `hashable` helper function to hash typically unhashable Python objects.
+
+    Parameters
+    ----------
+    obj : callable
+        the function to apply the caching to
+    bound : bool
+        indicates if the [obj] is a bound method (self as first argument)
+        For bound methods, the cache is kept in a `_cache` attribute on [self].
     """
     # this is declared not to be a bound method, so just attach new attr to obj
     if not bound:
@@ -40,7 +49,7 @@ def memoizer(*args, **kwargs):
             key = (hashable(args[1:]), hashable(kwargs))
             if not hasattr(args[0], "_cache"):
                 setattr(args[0], "_cache", collections.defaultdict(dict))
-                # do not add to cache regestry
+                # do not add to cache registry
             cache = getattr(args[0], "_cache")[obj.__name__]
         if key not in cache:
             cache[key] = obj(*args, **kwargs)
@@ -75,19 +84,26 @@ def __setstate__(self, state):
         self.__dict__.update(state)
 
 
-def hashable(a):
+def hashable(a) -> int:
     """
-    Turn some unhashable objects into hashable ones.
+    Hashes many kinds of objects, including some that are unhashable through the builtin `hash` function.
+    Lists and tuples are hashed based on their elements.
     """
     if isinstance(a, dict):
-        return hashable(tuple((hashable(a1), hashable(a2)) for a1, a2 in a.items()))
+        # first hash the keys and values with hashable
+        # then hash the tuple of int-tuples with the builtin
+        return hash(tuple((hashable(k), hashable(v)) for k, v in a.items()))
+    if isinstance(a, (tuple, list)):
+        # lists are mutable and not hashable by default
+        # for memoization, we need the hash to depend on the items
+        return hash(tuple(map(hashable, a)))
     try:
         return hash(a)
     except TypeError:
         pass
     # Not hashable >>>
     try:
-        return hash(pickle.dumps(a))
+        return hash(dill.dumps(a))
     except Exception:
         if hasattr(a, "__dict__"):
             return hashable(a.__dict__)

pymc3/tests/test_memo.py

@@ -11,21 +11,56 @@
 #   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #   See the License for the specific language governing permissions and
 #   limitations under the License.
+import numpy as np
+import pymc3 as pm
 
-from pymc3.memoize import memoize
+from pymc3 import memoize
 
 
-def getmemo():
-    @memoize
-    def f(a, b=("a")):
-        return str(a) + str(b)
+def test_memo():
+    def fun(inputs, suffix="_a"):
+        return str(inputs) + str(suffix)
+    inputs = ["i1", "i2"]
+    assert fun(inputs) == "['i1', 'i2']_a"
+    assert fun(inputs, "_b") == "['i1', 'i2']_b"
 
-    return f
+    funmem = memoize.memoize(fun)
+    assert hasattr(fun, "cache")
+    assert isinstance(fun.cache, dict)
+    assert len(fun.cache) == 0
+    
+    assert funmem(inputs) == "['i1', 'i2']_a"
+    assert funmem(inputs) == "['i1', 'i2']_a"
+    assert len(fun.cache) == 1
+    assert funmem(inputs, "_b") == "['i1', 'i2']_b"
+    assert funmem(inputs, "_b") == "['i1', 'i2']_b"
+    assert len(fun.cache) == 2
 
+    # add items to the inputs list (the list instance remains identical !!)
+    inputs.append("i3")
+    assert funmem(inputs) == "['i1', 'i2', 'i3']_a"
+    assert funmem(inputs) == "['i1', 'i2', 'i3']_a"
+    assert len(fun.cache) == 3
 
-def test_memo():
-    f = getmemo()
 
-    assert f("x", ["y", "z"]) == "x['y', 'z']"
-    assert f("x", ["a", "z"]) == "x['a', 'z']"
-    assert f("x", ["y", "z"]) == "x['y', 'z']"
+def test_hashing_of_rv_tuples():
+    obs = np.random.normal(-1, 0.1, size=10)
+    with pm.Model() as pmodel:
+        mu = pm.Normal("mu", 0, 1)
+        sd = pm.Gamma("sd", 1, 2)
+        dd = pm.DensityDist(
+            "dd",
+            pm.Normal.dist(mu, sd).logp,
+            random=pm.Normal.dist(mu, sd).random,
+            observed=obs,
+        )
+        print()
+        for freerv in [mu, sd, dd] + pmodel.free_RVs:
+            for structure in [
+                freerv,
+                dict(alpha=freerv, omega=None),
+                [freerv, []],
+                (freerv, []),
+            ]:
+                print(f"testing hashing of: {structure}")
+                assert isinstance(memoize.hashable(structure), int)

pymc3/tests/test_sampling.py

@@ -930,9 +930,7 @@ def test_shared(self):
 
         assert gen2["y"].shape == (draws, n2)
 
-    @pytest.mark.skipif(sys.platform.startswith("win"), reason="Stackoverflow error on Windows")
     def test_density_dist(self):
-
         obs = np.random.normal(-1, 0.1, size=10)
         with pm.Model():
             mu = pm.Normal("mu", 0, 1)