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

[lucianopaz]

This closes #3595

[rpgoldman]

With all due respect, I think this calls out the problem thatt we don't have a clear statetment of what datta types can be in our networks and counted as variables. This causes us to have lots of duplicate if-then-else's that are prone to incompleteness...
It would be nice, for example, if I could put

assert instanceof(x, PyMCEntity)

in my code, but AFAICT there is no such assertion I can make.
If we could do that, we could also try to come up with a typecase-style statement that would be guaranteed to be complete.

[lucianopaz]

You are right that we are leaving many types out of our checks in _draw_value. However, would a PyMCEntity help us here? The type checking is done to determine the appropriate method to call (i.e. value, get_value, random, theano.function), which are different for different types. We should be able to deal with anything theano can throw at us, any numpy array and any RV or Distribution that we have. In my case, I wasn't aware of theano.gof.graph.Constant's existence, and only thought of theano.tensor.Constant.

[rpgoldman]

You are right that we are leaving many types out of our checks in _draw_value. However, would a PyMCEntity help us here? The type checking is done to determine the appropriate method to call (i.e. value, get_value, random, theano.function), which are different for different types. We should be able to deal with anything theano can throw at us, any numpy array and any RV or Distribution that we have. In my case, I wasn't aware of theano.gof.graph.Constant's existence, and only thought of theano.tensor.Constant.

I agree that we should be able to deal with anything that can be thrown at us, but it's impossible to do that with any assurance unless we know what can be thrown at us, and speaking as a new-to-PyMC3 developer, I don't have that knowledge, and seems like I get it only by trial and error.

In particular, given a model, m, and a varname nm, it would be nice to have a clear understanding of what types of entity m[nm] can return. I think that's pm.FreeRV, pm.ObservedRV, and Theano.Variable (for Deterministic), but I can't swear to it. There's a similar uncertainty in other places in the code base. I'm trying to cope with that myself by adding type hints, but some more information would be helpful.

[lucianopaz]

m[nm] can return any of our RVs: free, transformed, observed and multiObserved. Then it can also be any theano kind of variable or part of a compute graph. I don't know all of the types theano can throw at us but at least I thought that they could be summarized into constants, shareds and variables, but apparently not.
Besides that, distribution parameters could also be numbers or numpy arrays.

I also got most of this from trial and error and some source code inspection. The type hints you are adding are very helpful!

[michaelosthege]

lgtm

[michaelosthege]

@lucianopaz if you can identify groups of types that should always be checked together, you could put them as a tuple somewhere. No need for a custom type hierarchy..

[lucianopaz]

Thanks for the idea @michaelosthege, I'll group the two kinds of constants in a tuple in vartypes and then merge