C++-11 features for unit tests?

[Daniel Lee]

Anyone care if the tests have C++-11 features? I'm wanting to use default template parameters for functions. This would make testing the distributions a lot easier and we might have a real chance at testing the distributions with the bounds included.

[Bob Carpenter]

It's OK by me.

It won't get in the way of an eventual CRAN-compatible
RStan (not that I've ever cared about that).

If we just give up on CRAN (or they start accepting
more modern code), we could use C++11 features in
the rest of Stan.

All the compilers that could handle our templates could
handle the C++11 features we were using for default
error policies in template functions.

- Bob

[Jiqiang Guo]

It is not a problem with RStan since the tests are not included.

--
Jiqiang

> --
>
>

[Ben Goodrich]

On Thursday, November 15, 2012 11:39:14 PM UTC-5, Daniel Lee wrote:

Anyone care if the tests have C++-11 features? I'm wanting to use default template parameters for functions. This would make testing the distributions a lot easier and we might have a real chance at testing the distributions with the bounds included.

This seems to be a bit more problematic on clang than I would have guessed. I didn't notice before because --std wasn't actually getting set on linux until now.

First, there is an issue where clang does not support __float128

http://llvm.org/bugs/show_bug.cgi?id=13530

but I got around that by doing -D__STRICT_ANSI__, as suggested. Then, I get an error in test-unit

clang++ -I src -I lib/eigen_3.1.2 -I lib/boost_1.52.0 -O0 -Wall -Wno-unused-function -Wno-tautological-compare -I ../../../../../lib/boost_1.52.0  -I../../../../../lib/eigen_3.1.2 -I ../../../../../src  -c -Wno-c++11-extensions -std=gnu++0x -D__STRICT_ANSI__ -I lib/gtest_1.6.0/include -I lib/gtest_1.6.0 src/test/file_includes/stan_gm_test.cpp -o test/file_includes/stan_gm.o
In file included from src/test/file_includes/stan_gm_test.cpp:1:
In file included from src/stan/gm.hpp:4:
In file included from src/stan/gm/ast_def.cpp:5:
In file included from lib/boost_1.52.0/boost/variant/recursive_variant.hpp:17:
In file included from lib/boost_1.52.0/boost/variant/detail/enable_recursive.hpp:21:
lib/boost_1.52.0/boost/mpl/eval_if.hpp:38:26: error: no type named 'type' in 'boost::result_of<boost::phoenix::actor<boost::spirit::attribute<1> > (boost::spirit::unused_type,
      boost::spirit::context<boost::fusion::cons<std::vector<stan::gm::var_decl, std::allocator<stan::gm::var_decl> > &, boost::fusion::cons<bool, boost::fusion::cons<int, boost::fusion::nil>
      > >, boost::fusion::vector1<bool> >)>'
    typedef typename f_::type type;
            ~~~~~~~~~~~~~^~~~

There is a giant error cascade from there, which I am attaching.

Ben

[Ben Goodrich]

On Tuesday, November 20, 2012 11:58:55 PM UTC-5, Ben Goodrich wrote:

On Thursday, November 15, 2012 11:39:14 PM UTC-5, Daniel Lee wrote:

Anyone care if the tests have C++-11 features? I'm wanting to use default template parameters for functions. This would make testing the distributions a lot easier and we might have a real chance at testing the distributions with the bounds included.

This seems to be a bit more problematic on clang than I would have guessed. I didn't notice before because --std wasn't actually getting set on linux until now.

I just tried test-unit with Clang's version of the C++ standard library. To do this, you need to install it and add

CFLAGS_GTEST += -std=c++0x
CFLAGS += -DGTEST_USE_OWN_TR1_TUPLE
CFLAGS += -stdlib=libc++

somewhere like make/os_*. But we get errors:

goodrich@CYBERPOWERPC:/opt/stan$ make clean-all && time make CC=clang++ O=0 test-unit
rm -f  
rm -f
rm -f -r models src/stan/model/model_header.hpp.gch src/stan/model/model_header.hpp.pch src/stan/model/model_header.hpp.d
rm -f -r doc/api
cd src/docs/stan-reference; rm -f *.aux *.bbl *.blg *.log *.toc *.pdf *.out *.idx *.ilg *.ind *.cb *.cb2 *.upa
rm -f -r demo
rm -f -r test bin doc
rm -f  
clang++ -I src -I lib/eigen_3.1.2 -I lib/boost_1.52.0 -O0 -Wall -Wno-unused-function -Wno-tautological-compare -DGTEST_USE_OWN_TR1_TUPLE -stdlib=libc++ -I ../../../../../lib/boost_1.52.0  -I../../../../../lib/eigen_3.1.2 -I ../../../../../src  -c -Wno-c++11-extensions -std=c++0x -I lib/gtest_1.6.0/include -I lib/gtest_1.6.0 lib/gtest_1.6.0/src/gtest-all.cc -o test/gtest.o
ar rv test/libgtest.a test/gtest.o
ar: creating test/libgtest.a
a - test/gtest.o
clang++ -I src -I lib/eigen_3.1.2 -I lib/boost_1.52.0 -O0 -Wall -Wno-unused-function -Wno-tautological-compare -DGTEST_USE_OWN_TR1_TUPLE -stdlib=libc++ -I ../../../../../lib/boost_1.52.0  -I../../../../../lib/eigen_3.1.2 -I ../../../../../src  -c -Wno-c++11-extensions -std=c++0x -I lib/gtest_1.6.0/include -I lib/gtest_1.6.0 src/test/io/csv_writer_test.cpp -o test/io/csv_writer.o
clang++ -I src -I lib/eigen_3.1.2 -I lib/boost_1.52.0 -O0 -Wall -Wno-unused-function -Wno-tautological-compare -DGTEST_USE_OWN_TR1_TUPLE -stdlib=libc++ -I ../../../../../lib/boost_1.52.0  -I../../../../../lib/eigen_3.1.2 -I ../../../../../src  -c -o bin/stan/agrad/agrad.o src/stan/agrad/agrad.cpp
clang++ -I src -I lib/eigen_3.1.2 -I lib/boost_1.52.0 -O0 -Wall -Wno-unused-function -Wno-tautological-compare -DGTEST_USE_OWN_TR1_TUPLE -stdlib=libc++ -I ../../../../../lib/boost_1.52.0  -I../../../../../lib/eigen_3.1.2 -I ../../../../../src  -c -o bin/stan/math/matrix.o src/stan/math/matrix.cpp
clang++ -I src -I lib/eigen_3.1.2 -I lib/boost_1.52.0 -O0 -Wall -Wno-unused-function -Wno-tautological-compare -DGTEST_USE_OWN_TR1_TUPLE -stdlib=libc++ -I ../../../../../lib/boost_1.52.0  -I../../../../../lib/eigen_3.1.2 -I ../../../../../src  -c -o bin/stan/agrad/matrix.o src/stan/agrad/matrix.cpp
In file included from src/stan/agrad/matrix.cpp:4:
In file included from src/stan/agrad/matrix.hpp:4:
In file included from src/stan/math/matrix.hpp:12:
In file included from lib/eigen_3.1.2/Eigen/Dense:1:
In file included from lib/eigen_3.1.2/Eigen/Core:22:
/usr/include/c++/v1/complex:672:19: error: no matching function for call to 'logb'
    _Tp __logbw = logb(fmax(fabs(__c), fabs(__d)));
                  ^~~~
lib/eigen_3.1.2/Eigen/src/Core/SelfCwiseBinaryOp.h:188:94: note: in instantiation of function template specialization 'std::__1::operator/<stan::agrad::var>' requested here
  tmp = PlainObject::Constant(rows(),cols(), NumTraits<Scalar>::IsInteger ? other : Scalar(1)/other);
                                                                                             ^
lib/eigen_3.1.2/Eigen/src/Core/Dot.h:155:9: note: in instantiation of member function 'Eigen::DenseBase<Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, -1, 0, -1, -1>, -1,
      1, true, true> >::operator/=' requested here
  *this /= norm();
        ^
lib/eigen_3.1.2/Eigen/src/Eigenvalues/EigenSolver.h:332:19: note: in instantiation of member function 'Eigen::MatrixBase<Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1,
      -1, 0, -1, -1>, -1, 1, true, true> >::normalize' requested here
      matV.col(j).normalize();
                  ^
src/stan/agrad/matrix.cpp:70:21: note: in instantiation of member function 'Eigen::EigenSolver<Eigen::Matrix<stan::agrad::var, -1, -1, 0, -1, -1> >::eigenvectors' requested here
      return solver.eigenvectors().real();
                    ^
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:135:13: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'double' for 1st argument;
__MATHCALL (logb,, (_Mdouble_ __x));
            ^
/usr/include/math.h:55:25: note: expanded from macro '__MATHCALL'
  __MATHDECL (_Mdouble_,function,suffix, args)
                        ^
/usr/include/math.h:57:22: note: expanded from macro '__MATHDECL'
  __MATHDECL_1(type, function,suffix, args); \
                     ^
/usr/include/math.h:65:31: note: expanded from macro '__MATHDECL_1'
  extern type __MATH_PRECNAME(function,suffix) args __THROW
                              ^
/usr/include/math.h:68:42: note: expanded from macro '__MATH_PRECNAME'
#define __MATH_PRECNAME(name,r) __CONCAT(name,r)
                                         ^
/usr/include/x86_64-linux-gnu/sys/cdefs.h:80:23: note: expanded from macro '__CONCAT'
#define __CONCAT(x,y)   x ## y
                        ^
/usr/include/c++/v1/cmath:1386:46: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'float' for 1st argument;
inline _LIBCPP_INLINE_VISIBILITY float       logb(float __x) _NOEXCEPT       {return logbf(__x);}
                                             ^
/usr/include/c++/v1/cmath:1387:46: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'long double' for 1st argument;
inline _LIBCPP_INLINE_VISIBILITY long double logb(long double __x) _NOEXCEPT {return logbl(__x);}
                                             ^
/usr/include/c++/v1/cmath:1392:1: note: candidate template ignored: substitution failure [with _A1 = stan::agrad::var]
logb(_A1 __x) _NOEXCEPT {return logb((double)__x);}
^
In file included from src/stan/agrad/matrix.cpp:4:
In file included from src/stan/agrad/matrix.hpp:4:
In file included from src/stan/math/matrix.hpp:12:
In file included from lib/eigen_3.1.2/Eigen/Dense:1:
In file included from lib/eigen_3.1.2/Eigen/Core:22:
/usr/include/c++/v1/complex:680:15: error: no matching function for call to 'scalbn'
    _Tp __x = scalbn((__a * __c + __b * __d) / __denom, -__ilogbw);
              ^~~~~~
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:293:13: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'double' for 1st argument;
__MATHCALL (scalbn,, (_Mdouble_ __x, int __n));
            ^
/usr/include/math.h:55:25: note: expanded from macro '__MATHCALL'
  __MATHDECL (_Mdouble_,function,suffix, args)
                        ^
/usr/include/math.h:57:22: note: expanded from macro '__MATHDECL'
  __MATHDECL_1(type, function,suffix, args); \
                     ^
/usr/include/math.h:65:31: note: expanded from macro '__MATHDECL_1'
  extern type __MATH_PRECNAME(function,suffix) args __THROW
                              ^
/usr/include/math.h:68:42: note: expanded from macro '__MATH_PRECNAME'
#define __MATH_PRECNAME(name,r) __CONCAT(name,r)
                                         ^
/usr/include/x86_64-linux-gnu/sys/cdefs.h:80:23: note: expanded from macro '__CONCAT'
#define __CONCAT(x,y)   x ## y
                        ^
/usr/include/c++/v1/cmath:1570:46: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'float' for 1st argument;
inline _LIBCPP_INLINE_VISIBILITY float       scalbn(float __x, int __y) _NOEXCEPT       {return scalbnf(__x, __y);}
                                             ^
/usr/include/c++/v1/cmath:1571:46: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'long double' for 1st argument;
inline _LIBCPP_INLINE_VISIBILITY long double scalbn(long double __x, int __y) _NOEXCEPT {return scalbnl(__x, __y);}
                                             ^
/usr/include/c++/v1/cmath:1576:1: note: candidate template ignored: substitution failure [with _A1 = stan::agrad::var]
scalbn(_A1 __x, int __y) _NOEXCEPT {return scalbn((double)__x, __y);}
^
In file included from src/stan/agrad/matrix.cpp:4:
In file included from src/stan/agrad/matrix.hpp:4:
In file included from src/stan/math/matrix.hpp:12:
In file included from lib/eigen_3.1.2/Eigen/Dense:1:
In file included from lib/eigen_3.1.2/Eigen/Core:22:
/usr/include/c++/v1/complex:681:15: error: no matching function for call to 'scalbn'
    _Tp __y = scalbn((__b * __c - __a * __d) / __denom, -__ilogbw);
              ^~~~~~
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:293:13: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'double' for 1st argument;
__MATHCALL (scalbn,, (_Mdouble_ __x, int __n));
            ^
/usr/include/math.h:55:25: note: expanded from macro '__MATHCALL'
  __MATHDECL (_Mdouble_,function,suffix, args)
                        ^
/usr/include/math.h:57:22: note: expanded from macro '__MATHDECL'
  __MATHDECL_1(type, function,suffix, args); \
                     ^
/usr/include/math.h:65:31: note: expanded from macro '__MATHDECL_1'
  extern type __MATH_PRECNAME(function,suffix) args __THROW
                              ^
/usr/include/math.h:68:42: note: expanded from macro '__MATH_PRECNAME'
#define __MATH_PRECNAME(name,r) __CONCAT(name,r)
                                         ^
/usr/include/x86_64-linux-gnu/sys/cdefs.h:80:23: note: expanded from macro '__CONCAT'
#define __CONCAT(x,y)   x ## y
                        ^
/usr/include/c++/v1/cmath:1570:46: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'float' for 1st argument;
inline _LIBCPP_INLINE_VISIBILITY float       scalbn(float __x, int __y) _NOEXCEPT       {return scalbnf(__x, __y);}
                                             ^
/usr/include/c++/v1/cmath:1571:46: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'long double' for 1st argument;
inline _LIBCPP_INLINE_VISIBILITY long double scalbn(long double __x, int __y) _NOEXCEPT {return scalbnl(__x, __y);}
                                             ^
/usr/include/c++/v1/cmath:1576:1: note: candidate template ignored: substitution failure [with _A1 = stan::agrad::var]
scalbn(_A1 __x, int __y) _NOEXCEPT {return scalbn((double)__x, __y);}
^
In file included from src/stan/agrad/matrix.cpp:4:
In file included from src/stan/agrad/matrix.hpp:4:
In file included from src/stan/math/matrix.hpp:12:
In file included from lib/eigen_3.1.2/Eigen/Dense:1:
In file included from lib/eigen_3.1.2/Eigen/Core:22:
/usr/include/c++/v1/complex:598:9: error: no matching function for call to 'isnan'
    if (isnan(__x) && isnan(__y))
        ^~~~~
lib/eigen_3.1.2/Eigen/src/Core/Functors.h:54:110: note: in instantiation of function template specialization 'std::__1::operator*<stan::agrad::var>' requested here
  EIGEN_STRONG_INLINE const result_type operator() (const LhsScalar& a, const RhsScalar& b) const { return a * b; }
                                                                                                             ^
lib/eigen_3.1.2/Eigen/src/Core/SelfCwiseBinaryOp.h:103:13: note: in instantiation of member function 'Eigen::internal::scalar_product_op<std::__1::complex<stan::agrad::var>,
      std::__1::complex<stan::agrad::var> >::operator()' requested here
      tmp = m_functor(tmp, _other.coeff(index));
            ^
lib/eigen_3.1.2/Eigen/src/Core/Assign.h:304:11: note: in instantiation of function template specialization
      'Eigen::SelfCwiseBinaryOp<Eigen::internal::scalar_product_op<std::__1::complex<stan::agrad::var>, std::__1::complex<stan::agrad::var> >,
      Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, -1, 0, -1, -1>, -1, 1, true, true>,
      Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> >
      >::copyCoeff<Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> > >'
      requested here
      dst.copyCoeff(i, src);
          ^
lib/eigen_3.1.2/Eigen/src/Core/SelfCwiseBinaryOp.h:137:61: note: in instantiation of member function
      'Eigen::internal::assign_impl<Eigen::SelfCwiseBinaryOp<Eigen::internal::scalar_product_op<std::__1::complex<stan::agrad::var>, std::__1::complex<stan::agrad::var> >,
      Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, -1, 0, -1, -1>, -1, 1, true, true>,
      Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> > >,
      Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> >, 1, 0, 0>::run'
      requested here
      internal::assign_impl<SelfCwiseBinaryOp, RhsDerived>::run(*this,rhs.derived());
                                                            ^
lib/eigen_3.1.2/Eigen/src/Core/Assign.h:522:97: note: in instantiation of function template specialization
      'Eigen::SelfCwiseBinaryOp<Eigen::internal::scalar_product_op<std::__1::complex<stan::agrad::var>, std::__1::complex<stan::agrad::var> >,
      Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, -1, 0, -1, -1>, -1, 1, true, true>,
      Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> >
      >::lazyAssign<Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> > >'
      requested here
  static EIGEN_STRONG_INLINE Derived& run(Derived& dst, const OtherDerived& other) { return dst.lazyAssign(other.derived()); }
                                                                                                ^
lib/eigen_3.1.2/Eigen/src/Core/Assign.h:562:59: note: (skipping 1 context in backtrace; use -ftemplate-backtrace-limit=0 to see all)
  return internal::assign_selector<Derived,OtherDerived>::run(derived(), other.derived());
                                                          ^
lib/eigen_3.1.2/Eigen/src/Core/SelfCwiseBinaryOp.h:150:20: note: in instantiation of function template specialization
      'Eigen::MatrixBase<Eigen::SelfCwiseBinaryOp<Eigen::internal::scalar_product_op<std::__1::complex<stan::agrad::var>, std::__1::complex<stan::agrad::var> >,
      Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, -1, 0, -1, -1>, -1, 1, true, true>,
      Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> > >
      >::operator=<Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> > >'
      requested here
      return Base::operator=(rhs);
                   ^
lib/eigen_3.1.2/Eigen/src/Core/SelfCwiseBinaryOp.h:188:7: note: in instantiation of member function
      'Eigen::SelfCwiseBinaryOp<Eigen::internal::scalar_product_op<std::__1::complex<stan::agrad::var>, std::__1::complex<stan::agrad::var> >,
      Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, -1, 0, -1, -1>, -1, 1, true, true>,
      Eigen::CwiseNullaryOp<Eigen::internal::scalar_constant_op<std::__1::complex<stan::agrad::var> >, Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, 1, 0, -1, 1> > >::operator='
      requested here
  tmp = PlainObject::Constant(rows(),cols(), NumTraits<Scalar>::IsInteger ? other : Scalar(1)/other);
      ^
lib/eigen_3.1.2/Eigen/src/Core/Dot.h:155:9: note: in instantiation of member function 'Eigen::DenseBase<Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1, -1, 0, -1, -1>, -1,
      1, true, true> >::operator/=' requested here
  *this /= norm();
        ^
lib/eigen_3.1.2/Eigen/src/Eigenvalues/EigenSolver.h:332:19: note: in instantiation of member function 'Eigen::MatrixBase<Eigen::Block<Eigen::Matrix<std::__1::complex<stan::agrad::var>, -1,
      -1, 0, -1, -1>, -1, 1, true, true> >::normalize' requested here
      matV.col(j).normalize();
                  ^
src/stan/agrad/matrix.cpp:70:21: note: in instantiation of member function 'Eigen::EigenSolver<Eigen::Matrix<stan::agrad::var, -1, -1, 0, -1, -1> >::eigenvectors' requested here
      return solver.eigenvectors().real();
                    ^
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:235:19: note: candidate function not viable: no known conversion from 'stan::agrad::var' to 'double' for 1st argument;
__MATHDECL_1 (int,isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
                  ^
/usr/include/math.h:65:31: note: expanded from macro '__MATHDECL_1'
  extern type __MATH_PRECNAME(function,suffix) args __THROW
                              ^
/usr/include/math.h:68:42: note: expanded from macro '__MATH_PRECNAME'
#define __MATH_PRECNAME(name,r) __CONCAT(name,r)
                                         ^
/usr/include/x86_64-linux-gnu/sys/cdefs.h:80:23: note: expanded from macro '__CONCAT'
#define __CONCAT(x,y)   x ## y
                        ^
/usr/include/c++/v1/cmath:424:1: note: candidate template ignored: substitution failure [with _A1 = stan::agrad::var]
isnan(_A1 __x) _NOEXCEPT
^
4 errors generated.
make: *** [bin/stan/agrad/matrix.o] Error 1

Ben

[Ben Goodrich]

On Wednesday, November 21, 2012 9:44:18 PM UTC-5, Ben Goodrich wrote:

I just tried test-unit with Clang's version of the C++ standard library. To do this, you need to install it and add

CFLAGS_GTEST += -std=c++0x
CFLAGS += -DGTEST_USE_OWN_TR1_TUPLE
CFLAGS += -stdlib=libc++

somewhere like make/os_*. But we get errors

Upon further review, these are due to using the .real() method in eigendecompositions. To me, things like

Eigen::EigenSolver<matrix_v> solver(m);
return solver.eigenvectors().real();

are problematic. Either m is symmetric, in which case the calling function should have used the _sym variants, which in turn utilize SelfAdjointEigenSolver, or m is not symmetric in which case just returning the real part is generally not the right answer. I think there are cases where all the eigenvalues happen to be real even though the matrix is asymmetric, but they are not likely to occur when m is a matrix_v and it is not like we are verifying that anyway.

Side note: Another argument against hard-coding vars as doubles is that we can't support densities that involve complex numbers. Not that there are millions of users clamoring for complex number support, but there will probably be one eventually.

Then, there are some parts like this

Eigen::SelfAdjointEigenSolver<matrix_v> solver(m,Eigen::EigenvaluesOnly);
return solver.eigenvalues().real();

where the .real() is just unnecessary because the .eigenvalues() of a SelfAdjointEigenSolver are real by construction.

If I fix all that, then test-unit only has a few (unique) errors, all of which seem to be due to io or gm (full log attached).

In file included from src/test/io/dump_test.cpp:1:
src/stan/io/dump.hpp:550:14: error: no viable conversion from 'std::__1::basic_istream<char>' to 'bool'
        bool ok = in_.get(c);
             ^    ~~~~~~~~~~
1 error generated.

In file included from src/test/io/cmd_line_test.cpp:1:
src/stan/io/cmd_line.hpp:190:16: error: no viable conversion from 'basic_istream<char, std::__1::char_traits<char> >' to 'bool'
        return s >> x;
               ^~~~~~

In file included from src/test/file_includes/stan_gm_test.cpp:1:
In file included from src/stan/gm.hpp:4:
In file included from src/stan/gm/ast_def.cpp:5:
In file included from lib/boost_1.52.0/boost/variant/recursive_variant.hpp:17:
In file included from lib/boost_1.52.0/boost/variant/detail/enable_recursive.hpp:21:
lib/boost_1.52.0/boost/mpl/eval_if.hpp:38:26: error: no type named 'type' in

'boost::result_of<boost::phoenix::actor<boost::spirit::attribute<1> > (boost::spirit::unused_type,
      boost::spirit::context<boost::fusion::cons<std::__1::vector<stan::gm::var_decl, std::__1::allocator<stan::gm::var_decl> > &, boost::fusion::cons<bool, boost::fusion::cons<int,

      boost::fusion::nil> > >, boost::fusion::vector1<bool> >)>'

    typedef typename f_::type type;
            ~~~~~~~~~~~~~^~~~

In file included from src/test/file_includes/stan_gm_test.cpp:1:
In file included from src/stan/gm.hpp:6:
In file included from src/stan/gm/compiler.hpp:6:
In file included from src/stan/gm/parser.hpp:8:
In file included from lib/boost_1.52.0/boost/spirit/include/qi.hpp:16:
In file included from lib/boost_1.52.0/boost/spirit/home/qi.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/action.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/action/action.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/meta_compiler.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/support/meta_compiler.hpp:19:
In file included from lib/boost_1.52.0/boost/proto/proto.hpp:12:
In file included from lib/boost_1.52.0/boost/proto/core.hpp:21:
In file included from lib/boost_1.52.0/boost/proto/fusion.hpp:24:
In file included from lib/boost_1.52.0/boost/fusion/include/transform.hpp:10:
In file included from lib/boost_1.52.0/boost/fusion/algorithm/transformation/transform.hpp:10:
In file included from lib/boost_1.52.0/boost/fusion/view/transform_view/transform_view.hpp:14:
In file included from lib/boost_1.52.0/boost/fusion/view/transform_view/transform_view_iterator.hpp:17:
lib/boost_1.52.0/boost/fusion/view/transform_view/detail/value_of_impl.hpp:36:74: error: no type named 'type' in
      'boost::mpl::apply<boost::fusion::detail::apply_transform_result<boost::spirit::detail::expand_arg<boost::spirit::context<boost::fusion::cons<std::__1::vector<stan::gm::var_decl,
      std::__1::allocator<stan::gm::var_decl> > &, boost::fusion::cons<bool, boost::fusion::cons<int, boost::fusion::nil> > >, boost::fusion::vector1<bool> > > >,
      boost::phoenix::actor<boost::spirit::attribute<1> >, mpl_::na, mpl_::na, mpl_::na, mpl_::na>'
                typedef typename mpl::apply<transform_type, value_type>::type type;
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~

In file included from src/test/file_includes/stan_gm_test.cpp:1:
In file included from src/stan/gm.hpp:6:
In file included from src/stan/gm/compiler.hpp:6:
In file included from src/stan/gm/parser.hpp:8:
In file included from lib/boost_1.52.0/boost/spirit/include/qi.hpp:16:
In file included from lib/boost_1.52.0/boost/spirit/home/qi.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/action.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/action/action.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/meta_compiler.hpp:15:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/domain.hpp:18:
lib/boost_1.52.0/boost/spirit/home/support/context.hpp:108:13: error: no matching constructor for initialization of 'boost::fusion::cons<stan::gm::var_decl &, boost::fusion::cons<bool,
      boost::fusion::cons<int, boost::fusion::nil> > >'
        ) : attributes(
            ^

In file included from src/test/file_includes/stan_gm_test.cpp:1:
In file included from src/stan/gm.hpp:6:
In file included from src/stan/gm/compiler.hpp:6:
In file included from src/stan/gm/parser.hpp:8:
In file included from lib/boost_1.52.0/boost/spirit/include/qi.hpp:16:
In file included from lib/boost_1.52.0/boost/spirit/home/qi.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/action.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/action/action.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/meta_compiler.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/support/meta_compiler.hpp:19:
In file included from lib/boost_1.52.0/boost/proto/proto.hpp:12:
In file included from lib/boost_1.52.0/boost/proto/core.hpp:21:
In file included from lib/boost_1.52.0/boost/proto/fusion.hpp:25:
In file included from lib/boost_1.52.0/boost/fusion/include/as_list.hpp:10:
In file included from lib/boost_1.52.0/boost/fusion/container/list/convert.hpp:11:
lib/boost_1.52.0/boost/fusion/container/list/detail/build_cons.hpp:51:50: error: no type named 'type' in
      'boost::fusion::result_of::value_of<boost::fusion::transform_view_iterator<boost::fusion::vector_iterator<const boost::fusion::vector<boost::phoenix::actor<boost::spirit::attribute<1>
      >, boost::phoenix::actor<boost::spirit::attribute<2> >, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_,
      boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>, 0>, boost::spirit::detail::expand_arg<boost::spirit::context<boost::fusion::cons<std::__1::vector<stan::gm::var_decl,
      std::__1::allocator<stan::gm::var_decl> > &, boost::fusion::cons<bool, boost::fusion::cons<int, boost::fusion::nil> > >, boost::fusion::vector1<bool> > > > >'
            typename result_of::value_of<First>::type v = *f;
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~

In file included from src/test/file_includes/stan_gm_test.cpp:1:
In file included from src/stan/gm.hpp:6:
In file included from src/stan/gm/compiler.hpp:6:
In file included from src/stan/gm/parser.hpp:8:
In file included from lib/boost_1.52.0/boost/spirit/include/qi.hpp:16:
In file included from lib/boost_1.52.0/boost/spirit/home/qi.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/action.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/action/action.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/qi/meta_compiler.hpp:14:
In file included from lib/boost_1.52.0/boost/spirit/home/support/meta_compiler.hpp:19:
In file included from lib/boost_1.52.0/boost/proto/proto.hpp:12:
In file included from lib/boost_1.52.0/boost/proto/core.hpp:21:
In file included from lib/boost_1.52.0/boost/proto/fusion.hpp:25:
In file included from lib/boost_1.52.0/boost/fusion/include/as_list.hpp:10:
In file included from lib/boost_1.52.0/boost/fusion/container/list/convert.hpp:11:
lib/boost_1.52.0/boost/fusion/container/list/detail/build_cons.hpp:51:50: error: no type named 'type' in
      'boost::fusion::result_of::value_of<boost::fusion::transform_view_iterator<boost::fusion::vector_iterator<const boost::fusion::vector<boost::phoenix::actor<boost::spirit::attribute<1>
      >, boost::phoenix::actor<boost::spirit::attribute<2> >, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_,
      boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>, 0>, boost::spirit::detail::expand_arg<boost::spirit::context<boost::fusion::cons<stan::gm::statement &,
      boost::fusion::cons<bool, boost::fusion::cons<int, boost::fusion::nil> > >, boost::fusion::vector0<void> > > > >'
            typename result_of::value_of<First>::type v = *f;
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~

clang++ -I src -I lib/eigen_3.1.2 -I lib/boost_1.52.0 -O0 -Wall -Wno-unused-function -Wno-tautological-compare -DGTEST_USE_OWN_TR1_TUPLE -stdlib=libc++ -I ../../../../../lib/boost_1.52.0  -I../../../../../lib/eigen_3.1.2 -I ../../../../../src -lpthread  -o src/test/gm/model_specs/function_signatures1 src/test/gm/model_specs/function_signatures1.o -Lbin -lstan
src/test/gm/model_specs/function_signatures1.o: In function `function_signatures1_model_namespace::function_signatures1_model::log_prob(std::__1::vector<stan::agrad::var, std::__1::allocator<stan::agrad::var> >&, std::__1::vector<int, std::__1::allocator<int> >&, std::__1::basic_ostream<char, std::__1::char_traits<char> >*)':
src/test/gm/model_specs/function_signatures1.cpp:(.text._ZN36function_signatures1_model_namespace26function_signatures1_model8log_probERNSt3__16vectorIN4stan5agrad3varENS1_9allocatorIS5_EEEERNS2_IiNS6_IiEEEEPNS1_13basic_ostreamIcNS1_11char_traitsIcEEEE[_ZN36function_signatures1_model_namespace26function_signatures1_model8log_probERNSt3__16vectorIN4stan5agrad3varENS1_9allocatorIS5_EEEERNS2_IiNS6_IiEEEEPNS1_13basic_ostreamIcNS1_11char_traitsIcEEEE]+0x8201): undefined reference to `stan::agrad::eigenvectors(Eigen::Matrix<stan::agrad::var, -1, -1, 0, -1, -1> const&)'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [src/test/gm/model_specs/function_signatures1] Error 1

Ben

[Bob Carpenter]

On 11/21/12 10:32 PM, Ben Goodrich wrote:
> On Wednesday, November 21, 2012 9:44:18 PM UTC-5, Ben Goodrich wrote:
>
> I just tried test-unit with Clang's version of the C++ standard library. To do this, you need to install it and add
>
> |
> CFLAGS_GTEST += -std=c++0x
> CFLAGS += -DGTEST_USE_OWN_TR1_TUPLE
> CFLAGS += -stdlib=libc++
> |
>
> somewhere like make/os_*. But we get errors
>
>
> Upon further review, these are due to using the .real() method in eigendecompositions. To me, things like
>
> |
> Eigen::EigenSolver<matrix_v> solver(m);
> return solver.eigenvectors().real();
> |
>
> are problematic. Either m is symmetric, in which case the calling function should have used the _sym variants, which in
> turn utilize SelfAdjointEigenSolver, or m is not symmetric in which case just returning the real part is generally not
> the right answer. I think there are cases where all the eigenvalues happen to be real even though the matrix is
> asymmetric, but they are not likely to occur when m is a matrix_v and it is not like we are verifying that anyway.




> Side note: Another argument against hard-coding vars as doubles is that we can't support densities that involve complex
> numbers. Not that there are millions of users clamoring for complex number support, but there will probably be one
> eventually.
>
> Then, there are some parts like this
>
> |
> Eigen::SelfAdjointEigenSolver<matrix_v> solver(m,Eigen::EigenvaluesOnly);
> return solver.eigenvalues().real();
> |
>
> where the .real() is just unnecessary because the .eigenvalues() of a |SelfAdjointEigenSolver| are real by construction.
>
> If I fix all that, then test-unit only has a few (unique) errors, all of which seem to be due to io or gm (full log
> attached).

I hacked it into its current state trying to get
it to compile.

Can you check in the fixes? I've been working
on the matrix libs, so a merge at this point may be
hairy. I'm happy to do it by hand if you send me the
fixes.

> In file included from src/test/io/dump_test.cpp:1:
> src/stan/io/dump.hpp:550:14: error: no viable conversion from 'std::__1::basic_istream<char>' to 'bool'
> bool ok = in_.get(c);
> ^ ~~~~~~~~~~
> 1 error generated.

Hmm -- std::istream::get(char& c) returns an istream
ref. The function it's in is never getting called, so
I just got rid of the entire function.

> |
> In file included from src/test/io/cmd_line_test.cpp:1:
> src/stan/io/cmd_line.hpp:190:16: error: no viable conversion from 'basic_istream<char, std::__1::char_traits<char> >' to
> 'bool'
> return s >> x;
> ^~~~~~

Yup, shouldn't be trying to convert this to bool.
Fixed.

> |
> In file included from src/test/file_includes/stan_gm_test.cpp:1:
> In file included from src/stan/gm.hpp:4:
> In file included from src/stan/gm/ast_def.cpp:5:
> In file included from lib/boost_1.52.0/boost/variant/recursive_variant.hpp:17:
> In file included from lib/boost_1.52.0/boost/variant/detail/enable_recursive.hpp:21:
> lib/boost_1.52.0/boost/mpl/eval_if.hpp:38:26: error: no type named 'type' in
> 'boost::result_of<boost::phoenix::actor<boost::spirit::attribute<1> > (boost::spirit::unused_type,
> boost::spirit::context<boost::fusion::cons<std::__1::vector<stan::gm::var_decl,
> std::__1::allocator<stan::gm::var_decl> > &, boost::fusion::cons<bool, boost::fusion::cons<int,
> boost::fusion::nil> > >, boost::fusion::vector1<bool> >)>'
> typedef typename f_::type type;
> ~~~~~~~~~~~~~^~~~

I just don't see a handle on our code here in the include
path, but then there's a stan::gm::var_decl in the error.
I'm also puzzled by the rest of the errors.
I wonder if there's some trait I'm not declaring somewhere?

I looked around on the web and found some similar errors,
but still don't understand what's going on.

I made a bunch of changes around this, so I'm going
to check in an intermediate state that doesn't touch
the main-line functionality of the parser, but does
add some types in the ast and some expressions in the parser.

- Bob

[Ben Goodrich]

On Thursday, November 22, 2012 1:09:27 AM UTC-5, Bob Carpenter wrote:

Can you check in the fixes?  I've been working
on the matrix libs, so a merge at this point may be
hairy.  I'm happy to do it by hand if you send me the
fixes.

I did so in 23f6bf4. However, part of that is commenting out the eigendecompositions for an asymmetric matrix, which are exposed and documented in the manual. So, we would have to decide what we really want to do here.

Ben

[Ben Goodrich]

In ca1fbbe, I uncommented my commenting out of those functions so that Jenkins doesn't freak out. You can revert ca1fbbe to avoid a few compiler errors with clang / libc++ / --std=c++0x

Ben

[Bob Carpenter]

Thanks. I'll see if I can get it integrated on
my side.

You have a much better handle on this than me.
What do you think we should do? I didn't even
realize the implications of asymmetry.

I'm happy to get rid of eigen decompositions for
asymmetric matrices and change the doc.

- Bob

[Ben Goodrich]

The only reason I can see to not get rid of it is because it is already there. But I doubt anyone is using it because it doesn't really make sense. It is rare to see eigenvalues being calculated in statistical models, rarer that the matrix in question is not positive definite, even rarer that the matrix is not symmetric, and I've never seen a statistical model where the relevant quantity is the real part of complex eigenvalues.

If / when we support complex parameters, it might make sense to have, but we don't currently.

Ben

[Bob Carpenter]

My linear algebra's wobbly at best -- I'm happy to
restrict to symmetric inputs to guarantee real-valued
results.

I see that you're using SelfAdjointEigenSolver. With that,
we could either test for symmetry (within some tolerance)
or just doc it as working on the replicated lower-triangular
matrix (which is what it does according to its doc). I'm
happy with either solution.

- Bob

> --
>
>

[Ben Goodrich]

On Thursday, November 22, 2012 2:55:07 PM UTC-5, Bob Carpenter wrote:

My linear algebra's wobbly at best -- I'm happy to
restrict to symmetric inputs to guarantee real-valued
results.

I see that you're using SelfAdjointEigenSolver.  With that,
we could either test for symmetry (within some tolerance)
or just doc it as working on the replicated lower-triangular
matrix (which is what it does according to its doc).  I'm
happy with either solution.

I don't think Eigen checks for SelfAdjointness, but Stan probably should. The cost of a symmetry check is going to be tiny relative to the cost of auto-diffing a tridiagonalization and a sequence of QR factorizations. It might even be the case that a JacobiSVD of a parameterized symmetric matrix is faster than a SelfAdjointEigenSolver when the auto-diffing is taken into account.

Ben

[Bob Carpenter]

This is different behavior rather than dropping
a check. It's what Eigen does.

Eigen doesn't check that the matrix coming in is
symmetric. In fact, it guarantees it won't look
above the diagonal, so all that can be empty (or
used for something else as their doc says).

So there are two different methods, really:

eigenvalues_sym(matrix,...);

eigenvalues_low_tri_self_adjoint(matrix,...);

First would test for symmetry in the matrix but only use
the lower triangular part. Second would only use the lower
triangular part.

- Bob

[Ben Goodrich]

On Thursday, November 22, 2012 2:47:51 PM UTC-5, Ben Goodrich wrote:

I'm happy to get rid of eigen decompositions for
asymmetric matrices and change the doc.

The only reason I can see to not get rid of it is because it is already there. But I doubt anyone is using it because it doesn't really make sense. It is rare to see eigenvalues being calculated in statistical models, rarer that the matrix in question is not positive definite, even rarer that the matrix is not symmetric, and I've never seen a statistical model where the relevant quantity is the real part of complex eigenvalues.

I'm not sure we should just disappear exposed functions. Can the parser be configured to give a deprecation message / suggested alternative if eig() appears in a .stan file?

Ben

[Bob Carpenter]

> 1) From Bob:

> I'm happy to get rid of eigen decompositions for
> asymmetric matrices and change the doc.
>

> 2) From Ben:

> The only reason I can see to not get rid of it is because it is already there. But I doubt anyone is using it
> because it doesn't really make sense. It is rare to see eigenvalues being calculated in statistical models, rarer
> that the matrix in question is not positive definite, even rarer that the matrix is not symmetric, and I've never
> seen a statistical model where the relevant quantity is the real part of complex eigenvalues.
>

> 3) From Ben:

> I'm not sure we should just disappear exposed functions. Can the parser be configured to give a deprecation message /
> suggested alternative if eig() appears in a .stan file?

I'm OK with just disappearing functions that don't make sense.
(See the second quote above for why it doesn't make sense.)
I think Ben's still on the fence. What's everyone else think?

I'm about to change the categorical distribution's behavior
to match the doc and treat outcomes as ranging from 1 to N instead
of the current implementation of 0 to (N-1). This is also a better
fit with the multinomial. This either fixes a bug or breaks backward
compatibility, depending on how you look at it. :-)

If I understand Ben's (3) above, we would leave eigenvalues() and
eigenvectors() as they were and print deprecation warnings if
they're used in a model. Ben -- will we need the .real() to pull
out the real part and if so, will that cause us problems? I'd not
bother with checking complex parts are all zero and printing a warning.
There's not a good way to do that in the current setup without throwing
an exception.

There's nothing in the parser now for deprecated functions,
but I could create a set of signatures to check during parsing.

- Bob

[Ben Goodrich]

On Sunday, November 25, 2012 4:45:52 PM UTC-5, Bob Carpenter wrote:

> 3)  From Ben:
> I'm not sure we should just disappear exposed functions. Can the parser be configured to give a deprecation message /
> suggested alternative if eig() appears in a .stan file?

I'm OK with just disappearing functions that don't make sense.
(See the second quote above for why it doesn't make sense.)
I think Ben's still on the fence.    What's everyone else think?

I'm about to change the categorical distribution's behavior
to match the doc and treat outcomes as ranging from 1 to N instead
of the current implementation of 0 to (N-1).  This is also a better
fit with the multinomial.  This either fixes a bug or breaks backward
compatibility, depending on how you look at it.  :-)

Breaks backward compatibility.

 

If I understand Ben's (3) above, we would leave eigenvalues() and
eigenvectors() as they were and print deprecation warnings if
they're used in a model.  Ben -- will we need the .real() to pull
out the real part and if so, will that cause us problems?  I'd not
bother with checking complex parts are all zero and printing a warning.
There's not a good way to do that in the current setup without throwing
an exception.

I'm pretty sure they don't make sense the way Stan is now. I doubt Stan even gets the auto-diff correct if some function utilizes complex arithmetic internally.

I was just saying that maybe if the parser sees

transformed parameters {
  vector[K] e;
  e <- eigenvalues(Sigma); // or eigenvectors()
}

that this should be a parser error of some kind. I don't really mind that much if people's old code gets broken as long as we point out how to fix it right there on the screen.  Alternatively, maybe the C++ versions of eigenvalues and eigenvectors can shout compiler errors like Eigen sometimes does with YOU ASKED FOR AN EIGENDECOMPOSITION OF A GENERAL MATRIX AND SHOULD CONSIDER USING THE SYMMETRIC VARIANTS IF POSSIBLE.

Ben