Uniform sampling of finitely-bounded variable in stan 2.10

[YannickJadoul]

Summary:

Using cmdstan 2.10.0, uniform sampling of a bounded variable real<lower=0, upper=1> x; fails to produce a random sample. The exact same code works in cmdstan 2.8.0, cmdstan 2.9.0 and pystan 2.9.0.

Am I going crazy (possible 😉), or did something get broken in 2.10?

Description:

Something seems to be going wrong in version 2.10.0 of (cmd)stan, if I sample the following model:
parameters { real<lower=0, upper=1> x; } model { x ~ uniform(0, 1); }

When I plot a histogram of the generated samples for x (given more than enough iteratations: ./test sample num_samples=1000000 num_warmup=10000), I see the following distribution:

The same doesn't happen in cmdstan 2.8.0, cmdstan 2.9.0 or pystan 2.9.0, so I am assuming it has got something to do with version 2.10 of stan:

Blue is cmdstan 2.8.0, green is 2.9.0, red is 2.10.0. Exactly the same small model, exactly the same parameters passed on the command line.

Reproducible Steps:

parameters { real<lower=0, upper=1> x; } model { x ~ uniform(0, 1); } and compiling with different versions of cmdstan is all it takes to reproduce. (If relevant: my C++ compiler version is g++ (Ubuntu 5.4.0-6ubuntu1~16.04.1) 5.4.0 20160609)

Current Version:

v2.10.0

[bob-carpenter]

I verified that this is a problem in RStan 2.10. And it doesn't matter whether there is the uniform sampling statement or not.

I don't think anything changed with 2.10 with respect to transforms, but we'll look into it. I assigned this to Daniel because he worked on the refactoring.

[bob-carpenter]

My guess is that this is missing a Jacobian adjustment somewhere. We need to check not just the upper and lower bounds, but all the constrained variables.

[syclik]

I just checked the generated code. There are 9 differences and they're all whitespace differences.

It's either a bug in the transform or the warmup / sampling changes that were introduced for v2.10.

[bob-carpenter]

It's sampling too far out in the tails on the unconstrained
space. It can't just be a missing Jacobian because that causes
Stan to run off into space and is really slow.

Any idea if it's producing the same lp__ values as in 2.9?

Also, any ideas how to add tests for this kind of thing? Ideally,
we should have simple tests of all the constraints. It'd require
an additional distribution if the uniform is improper.

• Bob

On Jul 25, 2016, at 5:36 PM, Daniel Lee notifications@github.com wrote:

I just checked the generated code. There are 9 differences and they're all whitespace differences.

It's either a bug in the transform or the warmup / sampling changes that were introduced for v2.10.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[davharris]

Re: testing

I remember John D Cook's chapter on testing RNGs having a helpful perspective. I found a copy online here (skip past the table of contents) http://www.johndcook.com/Beautiful_Testing_ch10.pdf

Dave

On Jul 25, 2016, at 6:06 PM, Bob Carpenter notifications@github.com wrote:

It's sampling too far out in the tails on the unconstrained
space. It can't just be a missing Jacobian because that causes
Stan to run off into space and is really slow.

Any idea if it's producing the same lp__ values as in 2.9?

Also, any ideas how to add tests for this kind of thing? Ideally,
we should have simple tests of all the constraints. It'd require
an additional distribution if the uniform is improper.

• Bob

On Jul 25, 2016, at 5:36 PM, Daniel Lee notifications@github.com wrote:

I just checked the generated code. There are 9 differences and they're all whitespace differences.

It's either a bug in the transform or the warmup / sampling changes that were introduced for v2.10.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[aadler]

If it helps, Luc Devroye's classic is also free online as a bunch of PDFs (See here). Probably irrelevant, but that histogram looks not too different from a beta with alpha beta < 1 (although it is too bowed and not flat enough in the center).

[betanalpha]

It's a small inflation that makes for a surprisingly large effect. I'm still investigating with limited wifi.

On Jul 25, 2016, at 18:06, Bob Carpenter notifications@github.com wrote:

It's sampling too far out in the tails on the unconstrained
space. It can't just be a missing Jacobian because that causes
Stan to run off into space and is really slow.

Any idea if it's producing the same lp__ values as in 2.9?

Also, any ideas how to add tests for this kind of thing? Ideally,
we should have simple tests of all the constraints. It'd require
an additional distribution if the uniform is improper.

• Bob

On Jul 25, 2016, at 5:36 PM, Daniel Lee notifications@github.com wrote:

I just checked the generated code. There are 9 differences and they're all whitespace differences.

It's either a bug in the transform or the warmup / sampling changes that were introduced for v2.10.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[betanalpha]

Found it. There's a copy/paste error in the main loop of transition -- right now the code tries to accept the new subtler twice because the accept code was pasted twice (or more accurately, I forgot to delete one version when I added the other). I haven't checked, but I'm guessing this will fix the other problematic model Ben had (smaller stepsizes -> higher acceptance probs where the double check becomes less of a problem).

I'm about to board this flight (and can't get the airport wifi to work anyways) so I'll patch tomorrow when I'm back in London (before the meeting). If there's more urgency then someone can just delete lines 110-113 in mcmc/hmc/nuts/base_nuts.hpp and then check that it fixes everything.

On Jul 25, 2016, at 18:06, Bob Carpenter notifications@github.com wrote:

It's sampling too far out in the tails on the unconstrained
space. It can't just be a missing Jacobian because that causes
Stan to run off into space and is really slow.

Any idea if it's producing the same lp__ values as in 2.9?

Also, any ideas how to add tests for this kind of thing? Ideally,
we should have simple tests of all the constraints. It'd require
an additional distribution if the uniform is improper.

• Bob

On Jul 25, 2016, at 5:36 PM, Daniel Lee notifications@github.com wrote:

I just checked the generated code. There are 9 differences and they're all whitespace differences.

It's either a bug in the transform or the warmup / sampling changes that were introduced for v2.10.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[YannickJadoul]

Confirming that removing these 4 lines indeed solves the problem.
Thank you all for the quick replies and solution!

Oh, and keep up the awesome work, of course! I'm becoming kind of a stan fan (if that is a thing)

[bob-carpenter]

Thanks --- Andrew's written extensively about testing MCMC
in particular with Cook (Samantha, not John) and Rubin. John
Cook's chapter focuses on simple univariate situations.

We're testing the _rng functions built into Stan using what
John Cook calls "bucket testing" (i.e., binning followed
by chi-square). We're not yet testing the covariances
of multivariate generators, and we really need to do that, too.

The difficulty is in the coding and integration. We need
a better end-to-end testing framework, which is complicated by
the multi-step compile then run on top of MCMC itself being
harder to test than simple MC.

• Bob

On Jul 25, 2016, at 6:22 PM, David J. Harris notifications@github.com wrote:

Re: testing

I remember John D Cook's chapter on testing RNGs having a helpful perspective. I found a copy online here (skip past the table of contents) http://www.johndcook.com/Beautiful_Testing_ch10.pdf

Dave

On Jul 25, 2016, at 6:06 PM, Bob Carpenter notifications@github.com wrote:

It's sampling too far out in the tails on the unconstrained
space. It can't just be a missing Jacobian because that causes
Stan to run off into space and is really slow.

Any idea if it's producing the same lp__ values as in 2.9?

Also, any ideas how to add tests for this kind of thing? Ideally,
we should have simple tests of all the constraints. It'd require
an additional distribution if the uniform is improper.

• Bob

On Jul 25, 2016, at 5:36 PM, Daniel Lee notifications@github.com wrote:

I just checked the generated code. There are 9 differences and they're all whitespace differences.

It's either a bug in the transform or the warmup / sampling changes that were introduced for v2.10.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[betanalpha]

https://github.com/stan-dev/stan/blob/feature/stat_valid_test/src/test/stat-validity/stat_valid_test_fixture.hpp

We just need a proper C++ API to call the sampler without using
any of the CmdStan code.

On Jul 26, 2016, at 3:39 PM, Bob Carpenter notifications@github.com wrote:

Thanks --- Andrew's written extensively about testing MCMC
in particular with Cook (Samantha, not John) and Rubin. John
Cook's chapter focuses on simple univariate situations.

We're testing the _rng functions built into Stan using what
John Cook calls "bucket testing" (i.e., binning followed
by chi-square). We're not yet testing the covariances
of multivariate generators, and we really need to do that, too.

The difficulty is in the coding and integration. We need
a better end-to-end testing framework, which is complicated by
the multi-step compile then run on top of MCMC itself being
harder to test than simple MC.

• Bob

On Jul 25, 2016, at 6:22 PM, David J. Harris notifications@github.com wrote:

Re: testing

I remember John D Cook's chapter on testing RNGs having a helpful perspective. I found a copy online here (skip past the table of contents) http://www.johndcook.com/Beautiful_Testing_ch10.pdf

Dave

On Jul 25, 2016, at 6:06 PM, Bob Carpenter notifications@github.com wrote:

It's sampling too far out in the tails on the unconstrained
space. It can't just be a missing Jacobian because that causes
Stan to run off into space and is really slow.

Any idea if it's producing the same lp__ values as in 2.9?

Also, any ideas how to add tests for this kind of thing? Ideally,
we should have simple tests of all the constraints. It'd require
an additional distribution if the uniform is improper.

• Bob

On Jul 25, 2016, at 5:36 PM, Daniel Lee notifications@github.com wrote:

I just checked the generated code. There are 9 differences and they're all whitespace differences.

It's either a bug in the transform or the warmup / sampling changes that were introduced for v2.10.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[aadler]

@bob-carpenter Have you thought about using the probability integral transform for testing?

[ericnovik]

@YannickJadoul Please, send me your mailing address and the size of your T-Shirt. Stan branded T will shortly be mailed to you. Thanks for uncovering this bug! (If you want specific kind, you can make your selection here: http://mc-stan.spreadshirt.com)

[bob-carpenter]

I don't even know what that is, at least not by that
name, but the issue is more with integration on the API
side, not the statistical method (see Michael's last post).
We'll be there soon---Daniel's nearly done with the command
refactor that will make this much easier.

• Bob

On Jul 26, 2016, at 1:14 PM, Avraham Adler notifications@github.com wrote:

@bob-carpenter Have you thought about using the probability integral transform for testing?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.