quick solution to Bounded sampling of arbitrary shapes

[brandonwillard]

Proposed fix for #1068.

[brandonwillard]

Just realized that this was left over from an attempt to make _random behave as it used to; specifically, when it took the size parameter. My initial implementation had samples take the shape size + tuple(self.shape), then code would produce the desired result from this method alone. However, after looking into the generating_samples function, I noticed that it takes steps to produce replications with shape given by size.

[springcoil]

I think this build stopped for some reason. I restarted it and we'll see what happens.

[springcoil]

$ python setup.py build_ext --inplace
running build_ext
$ THEANO_FLAGS='gcc.cxxflags="-march=core2"' nosetests $TESTCMD
nose.config: INFO: Ignoring files matching ['^.', '^_', '^setup.py$']
WARNING (theano.gof.cmodule): WARNING: your Theano flags gcc.cxxflags specify an -march=X flags.
It is better to let Theano/g++ find it automatically, but we don't do it now
test_bernoulli (pymc3.tests.test_distributions_random.BroadcastShape) ... ok (6.9192s)
test_beta (pymc3.tests.test_distributions_random.BroadcastShape) ... ok (7.0153s)
test_beta_binomial (pymc3.tests.test_distributions_random.BroadcastShape) ... ok (5.2186s)
test_binomial (pymc3.tests.test_distributions_random.BroadcastShape) ... ok (2.0908s)
test_bounded (pymc3.tests.test_distributions_random.BroadcastShape) ...
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

[brandonwillard]

This appears to be caused by the high rejection rate for the (new) default non-i.i.d. truncated sampler under the test dimensions.
To make this work, the tests could be made smaller, logic could be added that automatically determines the iid parameter (the tests are actually i.i.d.) and/or it could use a better--potentially not rejection-based--truncated normal sampler implementation.
The second approach (determining the iid parameter) sounds like the best, so I'll give that a shot.

[twiecki]

======================================================================
FAIL: test_bounded (pymc3.tests.test_distributions_random.ScalarParameterShape)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/pymc-devs/pymc3/pymc3/tests/test_distributions_random.py", line 132, in test_bounded
    self.check(BoundedNormal, mu=0., tau=1.)
  File "/home/travis/build/pymc-devs/pymc3/pymc3/tests/test_distributions_random.py", line 125, in check
    check_dist((dist, kwargs), test_cases)
  File "/home/travis/build/pymc-devs/pymc3/pymc3/tests/test_distributions_random.py", line 102, in check_dist
    check_shape(rv, size=size, expected=expected)
  File "/home/travis/build/pymc-devs/pymc3/pymc3/tests/test_distributions_random.py", line 114, in check_shape
    ' with `{3}` rv'.format(expected, actual, size, rv.distribution.__class__.__name__)
AssertionError: Expected shape `[5]` but got `(1,)` using `(size=5)` with `Bounded` rv

[brandonwillard]

Glad I added that new test! Looks like there's more I need understand about generate_samples.

In that test, the distribution has no shape and random is called with size=5. My expectation was that the steps in generate_samples would use replicate_samples to call _random/generator multiple times. This is apparently true when broadcast_shape and prefix_shape aren't used (or are effectively dimensionless). Otherwise, replications are delegated to the distribution's _random (called generator within generate_samples) implementation.

Most other RVs that call generate_samples have _random/generator implementations that take--and use--a size parameter. This touches on my earlier comments about multivariate distributions and i.i.d. assumptions: simply put, what are the contracts and/or assumptions relating to multivariate distributions and samples (specifically, within the sampling framework)?
Perhaps I'm not seeing clear distinctions (or relationships) between the size parameter, the concept of replication and its implementation in generate_samples, and multivariate supported distributions.

I could return to an earlier implementation that uses the size parameter in _random, but then I'd still have to make changes to generate_samples and/or how it's called.

[brandonwillard]

The features necessary to fix these issues are being pieced together in #1125.

[springcoil]

Any update on this?

[brandonwillard]

@springcoil, I've tied some important parts of this into PR #1125. Simply put, in order to properly sample multivariate truncated variables using accept/reject, we need to know the exact dimensions of the support and "size/dimension of the broadcast". See my recent comments in that PR for more details.

[twiecki]

Closing because of age. Feel free to reopen if you want to fix this up.