Type problem in MvNormal.logp when called with numpy array

[katosh]

Description of the problem

The multivariate distribution fails here:

import numpy as np
import pymc3 as pm
pm.MvNormal.dist(mu = np.array([0, 0]), cov = np.diag([1, 1])).logp(np.array([1, 1])).eval()

with the error

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-17-f2b19e774b86> in <module>()
      1 mvdist = pm.MvNormal.dist(mu = np.array([0, 0]), cov = np.diag([1, 1]))
      2 testData = np.array([1, 1])
----> 3 r = mvdist.logp(testData).eval()

~/Envs/noteEnv2/lib/python3.5/site-packages/pymc3/distributions/multivariate.py in logp(self, value)
    272     def logp(self, value):
    273         quaddist, logdet, ok = self._quaddist(value)
--> 274         k = value.shape[-1].astype(theano.config.floatX)
    275         norm = - 0.5 * k * pm.floatX(np.log(2 * np.pi))
    276         return bound(norm - 0.5 * quaddist - logdet, ok)

AttributeError: 'int' object has no attribute 'astype'

It seems value.shape is aspected to have the astype callable, like a numpy array but is only a python base tuple:

type(np.array([1, 1]).shape)
> tuple

Overwriting the shape by a numpy array did not do the trick:

testData = np.array([1, 1])
testData.shape = np.array(testData.shape)
type(testData.shape)
> tuple

Versions and main components

• PyMC3 Version: 3.4.1
• Theano Version: 1.0.5
• Python Version: 3.5.2
• Operating system: Ubuntu 16.04.4 LTS
• How did you install PyMC3: pip install git+https://github.com/pymc-devs/pymc3

[junpenglao]

Yes this is due to some implementation detail (I don't remember why) - try wrapping the observed value in theano: theano.shared(np.array([1.,1.]))

[katosh]

That worked. Thank you!

[michaelosthege]

re-opening because I just had the same problem.

The fix is easy, but I don't have the time right now to do the PR.

[AlexAndorra]

I can try working on this if you want @michaelosthege ?

[michaelosthege]

I can try working on this if you want @michaelosthege ?

Sure, contributions are always welcome :)

[AlexAndorra]

Great, I'll give it a try ASAP then -- will keep you posted if I have any questions 😉

[Ahanmr]

I was working on this PR, and I believe in master/pymc3/distributions/multivariate.py , the change needs to be made in logp function, which takes value as an argument, where,
k = value.shape[-1].astype(theano.config.floatX) needs to be changed to k = theano.shared(value).shape[-1].astype(theano.config.floatX) so that, instead of 'int', it is a <TensorType(int64, scalar)>. Can I make a PR?

[michaelosthege]

Maybe also k = pymc3.theanof.intX(value.shape[-1])

[rpgoldman]

Can anyone recall why it is legitimate to use value.shape[-1] here? The original author of this code presumably had a reason, but it's not documented why this is a reasonable thing to do.

[AlexAndorra]

Hi all, I don't want to play the killjoys but I wanted to kindly highlight that I signaled my interest in this issue last week.
No big deal as you're now further along than I was, but I think it's customary to give precedence to people who have publicly expressed interest -- or at least ask them where they are at.
That being said, thanks for your contribution and it seems like you did a good job in a short time period!

[aloctavodia]

@Ahanmr I agree with @AlexAndorra here, before starting to work on something you should check if someone else is working on that.

[Ahanmr]

@AlexAndorra @aloctavodia Sincere apologies, I'll keep that in mind next time, my only motive was to give it a go since I had an idea and thought I'd give it a try! Plus, I'm stuck with few tests failing, so please do help me with the same and correct me in case I'd made a mistake!