Merge pull request #3599 from lucianopaz/iss3595

Added theano.gof.graph.Constant into the type checks done in distributions.py

Author: lucianopaz

RELEASE-NOTES.md

@@ -24,6 +24,7 @@
 - Fixed a bug in `Categorical.logp`. In the case of multidimensional `p`'s, the indexing was done wrong leading to incorrectly shaped tensors that consumed `O(n**2)` memory instead of `O(n)`. This fixes issue [#3535](https://github.com/pymc-devs/pymc3/issues/3535)
 - Fixed a defect in `OrderedLogistic.__init__` that unnecessarily increased the dimensionality of the underlying `p`. Related to issue issue [#3535](https://github.com/pymc-devs/pymc3/issues/3535) but was not the true cause of it.
 - Wrapped `DensityDist.rand` with `generate_samples` to make it aware of the distribution's shape. Added control flow attributes to still be able to behave as in earlier versions, and to control how to interpret the `size` parameter in the `random` callable signature. Fixes [3553](https://github.com/pymc-devs/pymc3/issues/3553)
+- Added `theano.gof.graph.Constant` to type checks done in `_draw_value` (fixes issue [3595](https://github.com/pymc-devs/pymc3/issues/3595))
 
 
 ## PyMC3 3.7 (May 29 2019)

pymc3/distributions/distribution.py

@@ -9,7 +9,7 @@
     Model, get_named_nodes_and_relations, FreeRV,
     ObservedRV, MultiObservedRV, Context, InitContextMeta
 )
-from ..vartypes import string_types
+from ..vartypes import string_types, theano_constant
 from .shape_utils import (
     to_tuple,
     get_broadcastable_dist_samples,
@@ -92,7 +92,7 @@ def getattr_value(self, val):
         if isinstance(val, tt.TensorVariable):
             return val.tag.test_value
 
-        if isinstance(val, tt.TensorConstant):
+        if isinstance(val, theano_constant):
             return val.value
 
         return val
@@ -502,7 +502,7 @@ def __init__(self):
 def is_fast_drawable(var):
     return isinstance(var, (numbers.Number,
                             np.ndarray,
-                            tt.TensorConstant,
+                            theano_constant,
                             tt.sharedvar.SharedVariable))
 
 
@@ -592,7 +592,7 @@ def draw_values(params, point=None, size=None):
             if (next_, size) in drawn:
                 # If the node already has a givens value, skip it
                 continue
-            elif isinstance(next_, (tt.TensorConstant,
+            elif isinstance(next_, (theano_constant,
                                     tt.sharedvar.SharedVariable)):
                 # If the node is a theano.tensor.TensorConstant or a
                 # theano.tensor.sharedvar.SharedVariable, its value will be
@@ -783,7 +783,7 @@ def _draw_value(param, point=None, givens=None, size=None):
     """
     if isinstance(param, (numbers.Number, np.ndarray)):
         return param
-    elif isinstance(param, tt.TensorConstant):
+    elif isinstance(param, theano_constant):
         return param.value
     elif isinstance(param, tt.sharedvar.SharedVariable):
         return param.get_value()

pymc3/tests/test_random.py

@@ -91,6 +91,22 @@ def test_dep_vars(self):
                     np.all(val1 != val4), np.all(val2 != val3),
                     np.all(val2 != val4), np.all(val3 != val4)])
 
+    def test_gof_constant(self):
+        # Issue 3595 pointed out that slice(None) can introduce
+        # theano.gof.graph.Constant into the compute graph, which wasn't
+        # handled correctly by draw_values
+        n_d = 500
+        n_x = 2
+        n_y = 1
+        n_g = 10
+        g = np.random.randint(0, n_g, (n_d,))  # group
+        x = np.random.randint(0, n_x, (n_d,))  # x factor
+        with pm.Model():
+            multi_dim_rv = pm.Normal('multi_dim_rv', mu=0, sd=1, shape=(n_x, n_g, n_y))
+            indexed_rv = multi_dim_rv[x, g, :]
+            i = draw_values([indexed_rv])
+            assert i is not None
+
 
 class TestJointDistributionDrawValues(SeededTest):
     def test_joint_distribution(self):

pymc3/vartypes.py

@@ -1,20 +1,26 @@
-__all__ = ['bool_types', 'int_types', 'float_types', 'complex_types', 'continuous_types',
-           'discrete_types', 'typefilter', 'isgenerator']
-
-bool_types = set(['int8'])
-
-int_types = set(['int8',
-                 'int16',
-                 'int32',
-                 'int64',
-                 'uint8',
-                 'uint16',
-                 'uint32',
-                 'uint64'])
-float_types = set(['float32',
-                   'float64'])
-complex_types = set(['complex64',
-                     'complex128'])
+from theano.tensor import Constant as tensor_constant
+from theano.gof.graph import Constant as graph_constant
+
+
+__all__ = [
+    "bool_types",
+    "int_types",
+    "float_types",
+    "complex_types",
+    "continuous_types",
+    "discrete_types",
+    "typefilter",
+    "isgenerator",
+    "theano_constant",
+]
+
+bool_types = set(["int8"])
+
+int_types = set(
+    ["int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64"]
+)
+float_types = set(["float32", "float64"])
+complex_types = set(["complex64", "complex128"])
 continuous_types = float_types | complex_types
 discrete_types = bool_types | int_types
 
@@ -27,4 +33,7 @@ def typefilter(vars, types):
 
 
 def isgenerator(obj):
-    return hasattr(obj, '__next__')
+    return hasattr(obj, "__next__")
+
+
+theano_constant = (tensor_constant, graph_constant)