Named deterministics

[lucianopaz]

Fix for #2912.

The idea is to wrap the expression that is inputed to pm.Deterministic, adding the attribute pymc_name and changing the way in which the __getattribute__ and __setattr__ methods work. Both methods now inspect their call frame to infer whether the call came from inside or outside of the theano package. If the call is from inside theano, both methods work as usual, but from outside of theano, the way in which the name attribute is handled is changed. From outside theano, the name attribute will be handled as an alias for the pymc_name attribute. The added attribute, required some extra care to not break the __eq__, __ne__, __hash__, clone and clone_with_new_inputs methods. Furthermore, as the class is dynamically constructed, the __reduce__ method was changed in order to still be able to pickle models.

Finally, the NameWrapped class was intended to be used in a similar way as a decorator of a theano expression. In other words, my intention was to make NameWrapped(theano_var) return an instance of the wrapped class. This lead to an awkward design of the metaclass MetaNameWrapped, which sometimes calls __new__ twice during a __call__. This design's downside is that one cannot create a new instance (out) of the wrapped class from a preexisting instance (self) through the following signature:

out = self.__class__.__new__(self.__class__)

which is regularly used in theano.clone. I'm not super sure how to improve the current design, and if it is worth to do. Any comment is very welcome.

[twiecki]

pep8 required.

[twiecki]

@lucianopaz First of all, thanks so much for tackling this complex issue. I haven't taken a close look yet but on first skim it looks pretty daunting. That's not criticism per se as I'm sure it's required as you put quite some thought into this but it requires careful thought whether to add that much complexity and magic to the code base is worthwhile. I'll try to take a closer look soon.

[twiecki]

Also, we should make sure travis passes all tests.

[lucianopaz]

@twiecki, yes, I completely agree. What I did to get the naming like we wanted it to be, is very contrived. I couldn't think of a simpler solution that did not break something else. I understand if this PR ends up being dropped because of the added complexity.

I'm not really sure what is breaking the SMC tests. I made a small change and will commit it now to see if Travis gets fixed. If it doesn't, I'll take a closer look tomorrow.

[lucianopaz]

There's something weird going on with astroid on Travis.

  File "/home/travis/miniconda2/lib/python2.7/site-packages/astroid/bases.py", line 177, in igetattr
    attrs = self._proxied.igetattr(name, context, class_context=False)
TypeError: igetattr() got an unexpected keyword argument 'class_context'

For some reason the keyword argument passed to igetattr is class_context when it should be context. Do you have any idea how to fix that on Travis?

[lucianopaz]

@twiecki, it looks like the developers from from PyCQA have fixed the bug in astroid, which caused the test failures. The fix is available on their master branch, and will be available in the 1.6 release. What should we do? All tests pass on my local machine under python 2.7. I can try to see if the tests also pass locally under python 3.6.

[twiecki]

@lucaskolstad Can you pin that version for travis and see if everything else passes?

[junpenglao]

@lucianopaz I think we will likely drop this in favor of #3170. However, maybe it is possible to submit this to theano instead if it handles edge cases there?

[lucianopaz]

@junpenglao, I agree, @ColCarroll's fix is much simpler and solves the issue with the deterministic's names.