Add size everywhere

[ColCarroll]

A little more tidying up -- this makes a bunch more distributions work:

with pm.Model() as m:
    p = pm.Beta('p', 1., 1.)
pm.distributions.draw_values([m['p']], size=10)


[array([0.15474799, 0.20990774, 0.58038114, 0.34982708, 0.19272914,
        0.35373363, 0.7043671 , 0.64614833, 0.44618905, 0.95117071])]

with pm.Model() as m:
    p = pm.MvNormal('p', np.ones(2), np.eye(2), shape=2)
pm.distributions.draw_values([m['p']], size=10)

[array([[ 1.39565984,  1.03450569],
        [ 0.4672162 ,  1.27905665],
        [ 1.79445413,  1.30322014],
        [ 0.21250038,  1.16701847],
        [ 1.11164107,  1.84125069],
        [ 0.58553905,  3.00556503],
        [ 2.33114742,  1.86452748],
        [ 0.08479394, -0.44771028],
        [ 2.53627331,  1.11459445],
        [ 0.32806721,  2.16526088]])]

[ColCarroll]

Hrmph... this one test also isn't returning on my machine. Will check more tomorrow

[springcoil]

Ahh good stuff. Didn't realize you needed to add size there too.

…

On Sat, 19 May 2018, 10:29 pm Colin, ***@***.***> wrote: Hrmph... this one test also isn't returning on my machine. Will check more tomorrow — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2984 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA8DiAlT7K2EJuW5zo7DP9yqZK1Mq9_rks5t0ICjgaJpZM4UF0pW> .

[ColCarroll]

Update from a lot of checking is that this continues to be hard, and keeps stumbling into problems of size vs. shape (and occasionally repeat). Luckily, no core algorithms (yet) rely on size as an argument.

Specifically, something like

with pm.Model():
            a = pm.Uniform('a', lower=0, upper=1, shape=10)
            b = pm.Binomial('b', n=1, p=a, shape=10)

b.random(size=10)

still poses problems.

[junpenglao]

What about using size for things like a repeated pattern and restrict size as a positive integer, with shape being the broadcast parameter shape + repeat pattern. That way the behaviour of shape in distribution and RVs are unchanged, but doing random generation like prior sample will be fine (ie, broadcasting the RVs with shape into size+shape).

[springcoil]

This is a rabbit hole isn't it :) size and shape broadcasting seem to be incredibly tricky to get right.

[ColCarroll]

Updated this so that tests pass (fingers crossed) by removing the failing one (test_random_sample_returns_correctly). I do not think it was testing what was intended, and what was intended still doesn't work.

However, I do think that these makes the code base more correct, at least, and may help unblock #2983.

[ColCarroll]

The test that is failing appears to be skipped on all previous builds, but I do not see why it should be skipped. It appears to also get skipped on all non-3.6 environments in this build. It also fails on my local machine, at least for the last year.

Recreate with:

pytest -xv pymc3/tests/test_posteriors.py::TestNUTSLKJCholeskyCov

I can open a separate issue if I am not missing something obvious.

[aloctavodia]

@ColCarroll This used to work, but now fails.

    with pm.Model() as m:
        alfa = pm.HalfNormal('alfa', 20)
        p = pm.Beta('p', alfa, 1)
    pm.distributions.draw_values([m['p']], size=10)

Could we remove the size argument from generate_samples or that defeats the purpose of this PR?