Disallow scalar parameters in Dirichlet and Categorical

[neerajprad]

This adds a small check in Dirichlet and Categorical __init__ methods to ensure that scalar parameters are not admissible.

Motivation
Currently, Dirichlet throws no error when provided with a scalar parameter, but if we expand a scalar instance, it inherits the empty event shape from the original instance and gives unexpected results.

The alternative to this check is to promote event_shape to be torch.Size((1,)) if the original instance was a scalar, but that seems to add a bit more complexity (and changes the behavior of expand in that it would affect the event_shape as well as the batch_shape now). Does this seem reasonable? cc. @alicanb, @fritzo.

In [4]: d = dist.Dirichlet(torch.tensor(1.))

In [5]: d.sample()
Out[5]: tensor(1.0000)

In [6]: d.log_prob(d.sample())
Out[6]: tensor(0.)

In [7]: e = d.expand([3])

In [8]: e.sample()
Out[8]: tensor([0.3953, 0.1797, 0.4250])  # interpreted as events

In [9]: e.log_prob(e.sample())
Out[9]: tensor(0.6931)  # wrongly summed out

In [10]: e.batch_shape
Out[10]: torch.Size([3])

In [11]: e.event_shape
Out[11]: torch.Size([])  # cannot be empty

Additionally, based on review comments, this removes real_vector constraint. This was only being used in MultivariateNormal, but I am happy to revert this if we want to keep it around for backwards compatibility.

[alicanb]

Nice catch @neerajprad ! I think we should also add this to Categoricals as well. While browsing I also realized we have a real_vector constraint, used in MultivariateNormal. We can get rid of that since we already do .all() while validating args (just to be clear, i'm asking your opinion, not bikeshedding 😄 )

[neerajprad]

I think we should also add this to Categoricals as well.

Good point; I'll address it in this PR itself.

While browsing I also realized we have a real_vector constraint, used in MultivariateNormal. We can get rid of that since we already do .all() while validating args (just to be clear, i'm asking your opinion, not bikeshedding 😄 )

Hmm..I didn't quite know that we had a real_vector constraint as well! :) It seems like we are only using it in MultivariateNormal. Can we just remove the constraint altogether?

[fritzo]

IIRC the purpose of the real_vector constraint is to be able to correctly output a result of shape batch_shape from the .check() method, which is useful for verifying masked batches of distributions. If you think the real_vector constraint should be removed, please add an IndependentConstraint first, and ensure all transforms and bijections are correctly registered. This is sufficiently complex to warrant a separate PR.

[neerajprad]

Thanks for providing that context, @fritzo. I’ll remove the last commit. I don’t have a strong opinion on its removal, and will leave it until further discussion.

[neerajprad]

I think this should be good to merge, pending any further comments.

[facebook-github-bot]

soumith is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.