Error When Evaluating Log Likelihood for MvGaussianRandomWalk

[a3huang]

When I initialize a MvGaussianRandomWalk distribution using the cov keyword and try to evaluate the log likelihood as follows:

pm.MvGaussianRandomWalk.dist(mu=tt.constant([1,1]), cov=tt.constant(np.eye(2))).logp(tt.constant([[1,2]])).eval()

I get the following error:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-308-59dbcd30652a> in <module>()
----> 1 pm.MvGaussianRandomWalk.dist(mu=tt.constant([1,1]), cov=tt.constant(np.eye(2))).logp(tt.constant([[1,2]])).eval()

/Users/alexhuang/Library/Python/2.7/lib/python/site-packages/pymc3/distributions/timeseries.pyc in logp(self, x)
    357 
    358         innov_like = multivariate.MvNormal.dist(mu=x_im1 + self.mu, cov=self.cov,
--> 359                                                 tau=self.tau, chol=self.chol_cov).logp(x_i)
    360         return self.init.logp(x[0]) + tt.sum(innov_like)
    361 

/Users/alexhuang/Library/Python/2.7/lib/python/site-packages/pymc3/distributions/distribution.pyc in dist(cls, *args, **kwargs)
     45     def dist(cls, *args, **kwargs):
     46         dist = object.__new__(cls)
---> 47         dist.__init__(*args, **kwargs)
     48         return dist
     49 

/Users/alexhuang/Library/Python/2.7/lib/python/site-packages/pymc3/distributions/multivariate.pyc in __init__(self, mu, cov, tau, chol, lower, *args, **kwargs)
    220                  *args, **kwargs):
    221         super(MvNormal, self).__init__(mu=mu, cov=cov, tau=tau, chol=chol,
--> 222                                        lower=lower, *args, **kwargs)
    223         self.mean = self.median = self.mode = self.mu = self.mu
    224 

/Users/alexhuang/Library/Python/2.7/lib/python/site-packages/pymc3/distributions/multivariate.pyc in __init__(self, mu, cov, chol, tau, lower, *args, **kwargs)
     39             chol = chol.T
     40         if len([i for i in [tau, cov, chol] if i is not None]) != 1:
---> 41             raise ValueError('Incompatible parameterization. '
     42                              'Specify exactly one of tau, cov, '
     43                              'or chol.')

ValueError: Incompatible parameterization. Specify exactly one of tau, cov, or chol.

However, if I instead use the tau or chol keywords, it runs fine.

Digging into it, it looks like the __initCov__ function of _CovSet sets values for variables self.cov and self.chol_cov when we provide a covariance matrix via cov. However, this causes an error when we initialize the MvNormal in the logp function of MvGaussianRandomWalk because both cov and chol end up being passed into the initializer for MvNormal and then to _QuadFormBase which expects that only one of cov, tau, or chol is provided.

Is this the expected behavior?

• PyMC3 Version: 3.3
• Theano Version: 1.0.1
• Python Version: 2.7.12
• Operating system: MacOS 10.13.3
• How did you install PyMC3: pip

[junpenglao]

Thanks for reporting. @gBokiau is currently working on refactoring the MvNormal related implementation, and it should be fixed after #2847

[ricardoV94]

On master I am not getting an error:

pm.MvGaussianRandomWalk.dist(mu=tt.constant([1,1]), cov=tt.constant(np.eye(2))).logp(tt.constant([[1,2]])).eval()
array(0.)

But I am not sure if the output is correct. I get zero no matter what I put in the mu or logp:

pm.MvGaussianRandomWalk.dist(mu=tt.constant([10,1]), cov=tt.constant(np.eye(2)*5)).logp(tt.constant([[1,10]])).eval()
array(0.)

[kc611]

@ricardoV94 I think it's working as expected. MvGaussianRandomWalk initial distribution (init) defaults to Flat() basically giving a logp of zero over here (the xim1 and x_i are empty lists since inputs have only single dimension.)

https://github.com/pymc-devs/pymc3/blob/823906a3efcf66897eac8a4c89052d9153bca49e/pymc3/distributions/timeseries.py#L457-L463

Passing a distribution such as Normal to init args or having a x of more than one dimension gives a specific value(s) instead of array of zeroes.

[ricardoV94]

@kc611 That's what I was thinking, but thank you for confirming it. It would be useful to identify which commit fixed this issue and add a unit test that fails before and passes after.

[Sayam753]

The logp method for MvGaussianRandomWalk fails for multi-dimensional inputs as per #4388 (comment). There is a similar issue for GaussianRandomWalk as well #4010.

So, I suspect a major refactoring of these logp methods but I think it would make things simpler if efforts are redirected towards solving #4047 first for generalizing random walks.

[ricardoV94]

@Sayam753 I see, there is a much broader issue going on with the RandomWalk methods. Is there an issue that summarizes everything in one place (linking to all the separate issues)? That could be helpful to come back once the RandomWalk methods have been refactored.

[Sayam753]

I have these ideas -

• If we work on Add helper function for easier Random Walks specification #4047 , then potentially all shape related issues in GuassianRandomWalk fails for 2D shape #4010 and Add .random method to MvGaussianRandomWalk (Issue #4337) #4388 (comment) will get resolved along the way.
• Alongside random walks, we can also work on adding random methods to time series distributions (see .random() methods missing from some timeseries distributions #4337 ).
• If possible, can we figure out a way of generalizing test cases written for time series distributions? With this, it will become super easy to add a new time series class.

[kc611]

Has the pre-requisites to convert these RandomWalks into RandomVariable OPs been implemented (as mentioned in #4047) ? If so I think we should first start working on converting those into OPs. (I can help, maybe :-) ) Then we can easily figure out the a generalised approach for building RandomWalks as originally suggested in #4047. Otherwise any changes done here based on Distribution might turn obsolete.

(I don't know much about the cumsum() approach for creating RandomWalks though. So here I'm assuming talking about general direct implementations of these classes.)

[Sayam753]

Has the pre-requisites to convert these RandomWalks into RandomVariable OPs been implemented (as mentioned in #4047) ?

#4440 introduces major design changes for RandomVariable OP and is WIP. So, I guess it will be a good idea to follow the changes for better generalizing RandomWalks in near future.

The cumsum() method is used for building random samples for a RandomWalk where the noise is modelled by a distribution.

I'm assuming talking about general direct implementations of these classes.

Yes. A general implementation for RandomWalks would make sense.