Skip to content

GitLab

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
    • Switch to GitLab Next
  • Sign in / Register
eigen
eigen
  • Project overview
    • Project overview
    • Details
    • Activity
    • Releases
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 590
    • Issues 590
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
    • Iterations
  • Custom Issue Tracker
    • Custom Issue Tracker
  • Merge Requests 17
    • Merge Requests 17
  • Requirements
    • Requirements
    • List
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Test Cases
  • Operations
    • Operations
    • Incidents
    • Environments
  • Packages & Registries
    • Packages & Registries
    • Package Registry
    • Container Registry
  • Analytics
    • Analytics
    • CI / CD
    • Code Review
    • Insights
    • Issue
    • Repository
    • Value Stream
  • Snippets
    • Snippets
  • Members
    • Members
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • libeigen
  • eigeneigen
  • Issues
  • #2110

Closed
Open
Opened Jan 03, 2021 by Steve Bronder@SteveBronder

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.

Edited Jan 03, 2021 by Steve Bronder
Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
None
Due date
None
Reference: libeigen/eigen#2110