Possible Bug because const coeffRef not available in DenseCoeffsBase
I'm having a hard time replicating this as a minimal example, but for Stan we have an Eigen plugin for MatrixBase and ArrayBase to access the values and adjoints of our autodiff scalar type as a matrix ala
// var holds a vari pointer and is defined in pseudocode as
struct vari {
const double val_;
double adj_;
};
struct var {
vari* vi_;
};
// Making matrices and multiply values of A and adjoints of B
Eigen::Matrix<var, -1, -1> A = Eigen::MatrixXd::Random(4, 4);
Eigen::Matrix<var, -1, -1> B = Eigen::MatrixXd::Random(4, 4);
Eigen::Matrix<double, -1, -1> C = A.val() * B.adj();
The definitions of .val()
, .adj()
are available in eigen_plugins.h
with the ops defined in Eigen.hpp
mkdir stan
cd stan
git clone https://github.com/stan-dev/math
cd math
git checkout fix/eigen-internal-traits
./runTests.py ./test/unit/math/mix/fun/toss_me_test.cpp
Running the test above (which is the same as the example I posted above) gives the error in the gist here. The most notable parts of the error is at the end with the compiler complaining about passing a const CWiseUnaryView<>
in Gemm
. Though the actual error seems to trace down to not having a coeffRef() const
version in DenseCoeffsBase
. Though this is an odd error because Neither of the matrices in the example are const?
lib/eigen_3.3.9/Eigen/src/Core/products/GeneralMatrixMatrix.h:229:15: error: passing ‘const Eigen::CwiseUnaryView<Eigen::internal::val_ref_Op, Eigen::Matrix<stan::math::var_value<double>, -1, -1> >’ as ‘this’ argument discards qualifiers [-fpermissive]
&m_lhs.coeffRef(row,0), m_lhs.outerStride(),
In file included from lib/eigen_3.3.9/Eigen/Core:439,
from lib/eigen_3.3.9/Eigen/Dense:1,
from ./stan/math/prim/fun/Eigen.hpp:76,
from ./stan/math/prim/meta/append_return_type.hpp:4,
from ./stan/math/prim/meta.hpp:175,
from ./stan/math/fwd/core/fvar.hpp:4,
from ./stan/math/fwd/core.hpp:4,
from ./stan/math/mix/meta.hpp:6,
from ./stan/math/mix.hpp:4,
from ./test/unit/math/test_ad.hpp:4,
from test/unit/math/mix/fun/toss_me_test.cpp:1:
lib/eigen_3.3.9/Eigen/src/Core/DenseCoeffsBase.h:340:33: note: in call to ‘Eigen::DenseCoeffsBase<Derived, 1>::Scalar& Eigen::DenseCoeffsBase<Derived, 1>::coeffRef(Eigen::Index, Eigen::Index) [with Derived = Eigen::CwiseUnaryView<Eigen::internal::val_ref_Op, Eigen::Matrix<stan::math::var_value<double>, -1, -1> >; Eigen::DenseCoeffsBase<Derived, 1>::Scalar = double; Eigen::Index = long int]’
EIGEN_STRONG_INLINE Scalar& coeffRef(Index row, Index col)
Though this error goes away if I add the following const version of coeffRef()
to Eigen/src/Core/DenseCoeffsBase.h
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE const Scalar& coeffRef(Index row, Index col) const
{
eigen_internal_assert(row >= 0 && row < rows()
&& col >= 0 && col < cols());
return internal::evaluator<Derived>(derived()).coeffRef(row,col);
}
I've been trying to make .val()
and .adj()
conform to .real()
as much as I can as var
and complex
are a similar dual number scheme. For examples See scalar_real_op
vs. adj_Op
and scalar_real_ref_op
vs adj_ref_op
as well as their accessor member functions .real() const
vs .adj() const
and .real() nonconst
vs .adj() nonconst
.
I'm not sure if this is a me error or if the above const coeffRef()
needs to go into DenseCoeffsBase
. The godbolt link here shows that .real()
doesn't suffer from the above error as .val()
or .adj()
. I'm happy to make a PR to add the above to DenseCoeffsBase
though it feels like since .real()
works correctly this is an issue on my end.