Raise informative error message if MvNormal is instantiated without shape

[MarcoGorelli]

Currently, we have

>>> pm.MvNormal.dist(np.ones(2), np.eye(2)).random()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/marco/pymc3-dev/pymc3/distributions/multivariate.py", line 273, in random
    param = np.broadcast_to(param, shape=output_shape + dist_shape[-1:])
  File "<__array_function__ internals>", line 5, in broadcast_to
  File "/home/marco/miniconda3/envs/pymc3-dev-py38/lib/python3.8/site-packages/numpy/lib/stride_tricks.py", line 180, in broadcast_to
    return _broadcast_to(array, shape, subok=subok, readonly=True)
  File "/home/marco/miniconda3/envs/pymc3-dev-py38/lib/python3.8/site-packages/numpy/lib/stride_tricks.py", line 118, in _broadcast_to
    raise ValueError('cannot broadcast a non-scalar to a scalar array')
ValueError: cannot broadcast a non-scalar to a scalar array

which isn't a very clear error message. It could be improved to raise an informative error message if shape isn't passed. Furthermore, the line

        vals = pm.MvNormal('vals', mu=mu, chol=chol, observed=data)

from its docstring could be amended to include shape

---

Suggested by @Sayam753 here pymc-devs/pymc-examples#11

[Sayam753]

I am curious. @lucianopaz How does the observed parameter's shape interact with the distribution shape in PyMC3?

[lucianopaz]

@Sayam753, the behavior is very inconsistent. When you pass an observed value, the random variables that gets created is a view (i don't remember the op's exact name but it was something like viewOp) of the passed observations. This means that when you grab an rv's shape it will be a symbolic tensor that points to the shape of the observed. However, this doesn't change the distribution's shape so:

import numpy as np
import pymc3 as pm

with pm.Model():
    rv = pm.Normal('a', 0, 1, shape=(3,), observed=np.zeros(4))

print(rv.random().shape)  # returns (4,)
print(rv.distribution.random().shape) # returns (3,)

Ideally, we should only allow observations that are shaped as a batch of the distribution's shape (we did this in pymc-tfp but didn't back port it to pymc3).

[Sayam753]

Ideally, we should only allow observations that are shaped as a batch of the distribution's shape

Makes a lot of sense. Let me spend some time to figure out how this can be done in PyMC3.

[lucianopaz]

@Sayam753, i don't think it is worth the effort to check it out before v4

[Sayam753]

So, does that depend on the use of RandomVariable Op? (I do not know anything about it)

[lucianopaz]

What i mean to say is that whatever fix you work this problem now will have to be rethought once the random variables are replaced with an op

[ricardoV94]

In working in #4373 we decided to require the user to input the shape argument in the init method:

    ... < docstrings >
    shape : numerical tuple
        Describes shape of distribution. For example if n=array([5, 10]), and
        p=array([1, 1, 1]), shape should be (2, 3).
    """

    def __init__(self, n, a, shape, *args, **kwargs):
        super().__init__(shape, *args, **kwargs)

If you don't pass shape you get the default positional argument missing error, which should be informative enough for most Python users. So maybe something like that is sufficient here?

[Sayam753]

That's a good idea. Also, I think we need to have a check against passing shape as an empty tuple or None.

[mjhajharia]

can I take this up?

[canyon289]

Youre assigned @almostmeenal. Have at it

[ricardoV94]

I think this one is no longer relevant for V4

[canyon289]

If so can we close to avoid confusion?

[mjhajharia]

okay so I'll drop it then

…

On 17 Mar 2021, 7:48 PM +0530, Ravin Kumar ***@***.***>, wrote: If so can we close to avoid confusion? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ricardoV94]

Setting as wontfix as this may not be necessary after V4.

[ricardoV94]

Closing this one. I think this issue is no longer relevant following the use of RandomVariables in V4