Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shape Parameter Behavior Confusing for Normal and MvNormal #2848

Closed
a3huang opened this issue Feb 10, 2018 · 3 comments
Closed

Shape Parameter Behavior Confusing for Normal and MvNormal #2848

a3huang opened this issue Feb 10, 2018 · 3 comments

Comments

@a3huang
Copy link

a3huang commented Feb 10, 2018

The way the shape parameter behaves can be confusing and doesn't seem to be present in the docs.

For univariate Normal with scalar valued parameters, providing the shape parameter creates an array of the specified shape, which is I find intuitive.

pm.Normal.dist(mu=0, sd=1, shape=(2,5)).random().shape

For univariate Normal with vector valued parameters, the last dimension is broadcasted to match the rest of the dimensions of the given shape. I couldn’t find anywhere in the documentation that explicitly mentioned this and it took me while of playing around to figure out how the shape parameter behaved in this case.

pm.Normal.dist(mu=np.zeros(5,), sd=np.array(range(1, 6)), shape=(2,3,4,5)).random().shape

For MvNormal, it seems like the shape parameter doesn’t do anything at all. The following doesn’t raise any error and seemingly ignores the shape parameter, returning an array of shape (10,). This last one seems more like a bug to me.

pm.MvNormal.dist(mu=np.zeros(10,), cov=np.eye(10), shape=(33,41)).random().shape

Also, to be consistent with the behavior in the example with the univariate Normal with vector valued parameters, it seems to make sense to be able to write:

pm.MvNormal.dist(mu=np.zeros(10,), cov=np.eye(10), shape=(33,41,10)).random().shape

However, this raises the following error:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-133-8f8da38a9f24> in <module>()
----> 1 print pm.MvNormal.dist(mu=np.zeros(10,), cov=np.eye(10), shape=(33,41,10)).random().shape

/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)
     34         super(_QuadFormBase, self).__init__(*args, **kwargs)
     35         if len(self.shape) > 2:
---> 36             raise ValueError("Only 1 or 2 dimensions are allowed.")
     37 
     38         if chol is not None and not lower:

ValueError: Only 1 or 2 dimensions are allowed.

I think it would make this library much more user friendly if we explicitly documented these “gotchas”.

  • 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
Copy link
Member

Thanks for the comment, you are right: currently, the shape of RV is one of our recurrent issues and we see a lot of related corner cases. Hopefully it will get address in #2833

@lucianopaz
Copy link
Contributor

There are two sides to this issue:

  1. Make a small doc on the shape handling
  2. Fix the MvNormal shape handling

I'll look into the MvNormal's problem soonish.

@AlexAndorra
Copy link
Contributor

To be more explicit: this is is still an open issue, but this specific one was closed because it's related to #3706

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants