"discontinuous posterior" warning cause

[Matt Hoffman]

Hi all,

People have been getting the "is your posterior discontinuous?" error
message a lot more than I would expect given random initializations.
The reason is that find_reasonable_parameters() was being called
before the parameters were initialized, so it was starting with all
parameters=0.

This bug (which I just pushed a fix for) sometimes results in bad
behavior. I discovered it while playing with the Cauchy distribution
like so:

{{{
parameters {
real x;
real y;
}
transformed parameters {
real xovery;
xovery <- x / y;
}
model {
real z;

x ~ normal(0, 1);
y ~ normal(0, 1);

z <- 0;
z ~ normal(xovery, 1);
}
}}}

Before, the model crashed, because it was trying to find a reasonable
epsilon at the setting x=0,y=0 (which yields a nan). Now it works
fine, because that setting has 0 posterior mass.

BTW, the posterior for x and y in this model (plot attached) looks
like the top of an opened pre-pull-tab beer can.

Matt

[Ben Goodrich]

On Monday, October 15, 2012 9:56:51 PM UTC-4, Matt Hoffman wrote:

This bug (which I just pushed a fix for) sometimes results in bad
behavior.

This patch adds a couple of asserts(), and we already had a few others:

goodrich@CYBERPOWERPC:/opt/stan$ git grep -n "assert(" src/ | cat
src/stan/mcmc/adaptive_cdhmc.hpp:153:        assert(x.size() == _x.size());
src/stan/mcmc/adaptive_cdhmc.hpp:154:        assert(z.size() == _z.size());
src/stan/mcmc/hmc_base.hpp:142:          assert(params_r->size() == model.num_params_r());
src/stan/mcmc/hmc_base.hpp:146:          assert(params_i->size() == model.num_params_i());
src/stan/mcmc/hmc_base.hpp:168:        assert(x.size() == this->_x.size());
src/stan/mcmc/hmc_base.hpp:169:        assert(z.size() == this->_z.size());
src/stan/meta/traits.hpp:150:    // assert((length(x1) == 1) || (length(x1) == result));
src/stan/meta/traits.hpp:151:    // assert((length(x2) == 1) || (length(x2) == result));
src/stan/meta/traits.hpp:160:    // assert((length(x1) == 1) || (length(x1) == result));
src/stan/meta/traits.hpp:161:    // assert((length(x2) == 1) || (length(x2) == result));
src/stan/meta/traits.hpp:162:    // assert((length(x3) == 1) || (length(x3) == result));
src/test/io/reader_test.cpp:680:  assert(solver.eigenvalues()[0] > 1E-10);
src/test/io/reader_test.cpp:706:  assert(solver.eigenvalues()[0] > 1E-10);
src/test/io/reader_test.cpp:769:  assert(solver.eigenvalues()[0] > 1E-10); // check positive definite with smallest eigenvalue > 0
src/test/io/reader_test.cpp:795:  assert(solver.eigenvalues()[0] > 1E-10); // check positive definite with smallest eigenvalue > 0

I am not sure what to throw in this case (Daniel?), but if an assert() is triggered it would take down the process if it were called from R. I think asserts get disabled if NDEBUG is defined, but we should probably replace these with something else to be on the safe side.

Ben

[Bob Carpenter]

In general, assert() should be used to catch programmer error,
not to trap user errors. It's then typical to turn them off
"in production".

1. Calls to assert() in tests should be replaced with EXPECT_TRUE calls.

2. Calls to assert() elsewhere should be replaced with

a) runtime checks that throw exceptions if they are the
result of user behavior, or

b) nothing if they are programmer error.

An alternative would be to leave the asserts for case 2b in
place and run with -DNDEBUG option.

I'll clean these up before the next release. Any thoughts
on whether to just systematically remove all uses of assert()
or not?

- Bob

> --
>
>