model error checking for undefined transformed params

[Bob Carpenter]

Daniel was running into segfaults from models with
undefined transformed params. It's a programming error,
but we want to trap it and dump out an informative warning
message, not segfault!

So I added a test to the generated model code.
Every log-prob-gradient eval, it checks each
transformed parameter to make sure it was defined. If not,
it throws an exception, which halts the model and reports
what went wrong.

An alternative would be to initialize then and initialize
to NaN or 0 or something. But it seemed better to flag it.
Otherwise, the variables go through and have 0 variance in
the output and so mess up our convergence monitoring.

The model tests behave in a way I didn't expect if this
kind of exception crops up. Daniel -- try declaring, but
not definining a transformed parameter and see what happens.

I added a method is_undefined() to stan::agrad::var
along with a test. It'll succeed if you do something
like this:

var x;
x.is_uninitialized(); // evaluates to true
x = 5;
x.is_unitialized(); // evaluates to false

I also made sure to initialize a var's vi_ variable
to static_cast<vari*>(0) in the nullary constructor.

- Bob

[Daniel Lee]

Thanks for taking care of that!

We have an issue with the generated code for
models/basic_distributions/wishart2x2. It doesn't compile.

Both g++ and clang++ report two errors that I haven't gotten my head
around yet. I've snipped the two errors (from g++) and included it
below. clang++ gives back a similar message, but at least the output
has color.




g++ message:
g++ -I src -I lib/eigen_3.1.0 -I lib/eigen_3.1.0/unsupported -I
lib/boost_1.50.0 -O3 -Wall -Wno-unused-function -c -o
models/basic_distributions/wishart2x2.o
models/basic_distributions/wishart2x2.cpp
In file included from src/stan/gm/command.hpp:19:0,
from src/stan/model/model_header.hpp:20,
from models/basic_distributions/wishart2x2.cpp:3:
...
models/basic_distributions/wishart2x2.cpp:140:33: error: no match for
'operator[]' in
'((Eigen::DenseCoeffsBase<Eigen::Matrix<stan::agrad::var,
-0x00000000000000001, -0x00000000000000001, 0, -0x00000000000000001,
-0x00000000000000001>,
1>*)((Eigen::DenseBase<Eigen::Matrix<stan::agrad::var,
-0x00000000000000001, -0x00000000000000001, 0, -0x00000000000000001,
-0x00000000000000001> >*)(& W)))->Eigen::DenseCoeffsBase<Derived,
1>::operator[] [with Derived = Eigen::Matrix<stan::agrad::var,
-0x00000000000000001, -0x00000000000000001, 0, -0x00000000000000001,
-0x00000000000000001>, Eigen::DenseCoeffsBase<Derived, 1>::Scalar =
stan::agrad::var, Eigen::DenseCoeffsBase<Derived, 1>::Index = long
int](((long int)i0__))[i1__]'
...
lib/eigen_3.1.0/Eigen/src/Core/DenseCoeffsBase.h: In member function
'Eigen::DenseCoeffsBase<Derived, 1>::Scalar&
Eigen::DenseCoeffsBase<Derived,
1>::operator[](Eigen::DenseCoeffsBase<Derived, 1>::Index) [with
Derived = Eigen::Matrix<stan::agrad::var, -0x00000000000000001,
-0x00000000000000001, 0, -0x00000000000000001, -0x00000000000000001>,
Eigen::DenseCoeffsBase<Derived, 1>::Scalar = stan::agrad::var,
Eigen::DenseCoeffsBase<Derived, 1>::Index = long int]':
models/basic_distributions/wishart2x2.cpp:140:27: instantiated from here
lib/eigen_3.1.0/Eigen/src/Core/DenseCoeffsBase.h:390:7: error:
'THE_BRACKET_OPERATOR_IS_ONLY_FOR_VECTORS__USE_THE_PARENTHESIS_OPERATOR_INSTEAD'
is not a member of 'Eigen::internal::static_assertion<false>'

[Bob Carpenter]

On 7/12/12 10:42 PM, Daniel Lee wrote:
> Thanks for taking care of that!
>
> We have an issue with the generated code for
> models/basic_distributions/wishart2x2. It doesn't compile.

OK -- I can patch that later today. I introduced the bug
with an overly simplified accessor loop. The huge pain
in all of this code is that Eigen doesn't support
an operator[](int) for matrices that returns a row reference.
If it did, our life would be much easier in that
Eigen::Matrix<T,...> would work like vector<vector<T> >.

- Bob

[Ben Goodrich]

Why would we be using [] for accessing anyway? I think it is always better to use () or .coef() or .coefRef().

Ben

[Bob Carpenter]

We shouldn't be. We're already checking bounds,
so there's no point in doing it twice. (We're
still working on how to turn off our bounds
checking.)

I just keep wishing Eigen used [] like std::vector
so the code didn't have to have a three-way branch
at the end (or one before the end in the case of
matrices) of every recursion over dimensions.

- Bob

[Ben Goodrich]

You might be able to redefine the [] operator via the plugin mechanism if you feel that strongly about it, but I would say just don't use it.

Ben

[Bob Carpenter]

On 7/14/12 4:26 PM, Ben Goodrich wrote:
> You might be able to redefine the [] operator via the plugin mechanism if you feel that strongly about it, but I would
> say just don't use it.

I agree it's best not to use it, primarily to (a) be clear on
what's Eigen and what's std::vector, and (b) to prevent
ever using operator()[int] on a matrix by mistake (as in
that last bug).

From what I can tell from the doc:

http://eigen.tuxfamily.org/dox/QuickRefPage.html

operator[](int) is just a synonym for operator()(int). Both
do range checking unless NDEBUG nor EIGEN_NO_DEBUG is defined.

I suspect you'd get a duplicate operator definition if you
tried to inject operator[](int) using their plugin include.

There are probably still places in the code where
I do use operator[](int). It simplifies some of the
code by letting me template out std and Eigen vectors

std::vector<double>,
Eigen::Matrix<double,Dynamic,1>, and
Eigen::Matrix<double,1,Dynamic>.

The problem, of course, is Eigen::Matrix<double,Dynamic,Dynamic>,
which really must be accessed with either row(i) or (i,j).

- Bob