Last dimension is dropped by random when size is one

[huard]

Description of your problem

The last dimension is squeezed when generating random samples. This creates a problem when converting posterior predictive samples to DataArray objects.

In the following example, I would expect the second case to have shape (10, 1, 2, 1).

>>> pm.distributions.Uniform.dist(lower=np.zeros((1,2,3)), upper=np.ones((1,2,3))).random(size=10).shape
(10, 1, 2, 3)

>>> pm.distributions.Uniform.dist(lower=np.zeros((1,2,1)), upper=np.ones((1,2,1))).random(size=10).shape
(10, 1, 2)

Versions and main components

• PyMC3 Version: 3.8
• Theano Version: 1.0.4
• Python Version: 3.6.8
• Operating system: Ubuntu
• How did you install PyMC3: pip

[twiecki]

Hey David, I'll ping @lucianopaz -- wizard of shapes.

Thanks for PyMC2 :).

[lucianopaz]

@huard, @twiecki, I became aware of this some time ago, while refactoring pymc3.distributions.distribution.generate_samples. When the last axis of a distribution's shape is 1, or when we ask for random(..., size=1) of a scalar distribution, the last singleton axis is dropped. I'm totally against dropping these axis, but it is something that pymc3 has always done, and it is tested to do so (for some reason, probably regarding samples from scalar distributions). I'm in favor of changing generate_samples to prevent it from dropping the singleton axis, the main challenge with this will be to refactor all the tests that currently explicitly test that random(..., size=1) of a scalar distribution returns a scalar, instead of an array of shape (1,).

[huard]

@lucianopaz Thanks for the explanation. If you make the fix, I can then spend some time adjusting the tests.

@twiecki You're welcome, fun times !

[ricardoV94]

#4214 seems to have fixed this. I can reproduce before but not after.

Feel free to re-open if the problem subsists.

[twiecki]

Nice!

[MarcoGorelli]

Nice that it's fixed, thanks for checking @ricardoV94 !

Reopening though as it would be good to include this little example as a test case to make sure this continues working in the future, and this makes for a good beginner-friendly issue

[ricardoV94]

@MarcoGorelli that's a good point. I assumed the new tests after that PR would cover this issue but that is not necessarily the case (even less so since the issue was not even linked)

[canyon289]

@almostmeenal Assigning you to this one per your comment here #4214

Quick tip: Its better to comment on the issue you want to work on, rather than a related PR, as this issue would be the more relevant thread of discussion

Edit. Seems that github is not allowing me to do so but consider yourself assigned.

[mjhajharia]

@almostmeenal Assigning you to this one per your comment here #4214

Quick tip: Its better to comment on the issue you want to work on, rather than a related PR, as this issue would be the more relevant thread of discussion

Edit. Seems that github is not allowing me to do so but consider yourself assigned.

cool, thanks.