2

I'm having some trouble when using coeffRef() with a CWiseUnaryView function, but only when the function is declared as const

Reproducible example:

#include <Eigen/Core>

struct dummy_Op {
  EIGEN_EMPTY_STRUCT_CTOR(dummy_Op)
  EIGEN_DEVICE_FUNC
  EIGEN_STRONG_INLINE const double& 
    operator()(const double &v) const { return v; }
  EIGEN_DEVICE_FUNC
  EIGEN_STRONG_INLINE double& 
    operator()(double &v) const { return v; }
};

void foo(Eigen::MatrixXd &out)
{
    //Compiles
    Eigen::CwiseUnaryView<dummy_Op, Eigen::MatrixXd> view(out);
    view.coeffRef(0,0);

    //Doesn't Compile
    const Eigen::CwiseUnaryView<dummy_Op, Eigen::MatrixXd> const_view(out);
    const_view.coeffRef(0,0);
}

Returns:

<source>: In function 'void foo(Eigen::MatrixXd&)':
<source>:21:28: error: passing 'const Eigen::CwiseUnaryView<dummy_Op, 
Eigen::Matrix<double, -1, -1> >' as 'this' argument discards qualifiers 
[-fpermissive]

     const_view.coeffRef(0,0);

                            ^

In file included from /opt/compiler-explorer/libs/eigen/v3.3.4/Eigen/Core:413,
                 from <source>:1:
/opt/compiler-explorer/libs/eigen/v3.3.4/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<dummy_Op, Eigen::Matrix<double, 
-1, -1> >; Eigen::DenseCoeffsBase<Derived, 1>::Scalar = double; Eigen::Index = long int]'

     EIGEN_STRONG_INLINE Scalar& coeffRef(Index row, Index col)

                                 ^~~~~~~~

Compiler returned: 1

Compiler explorer: https://godbolt.org/z/kPHPuC

The side-effect of this, is that the multiplication of two (non-const) CWiseUnaryViews also fails, see example here: https://godbolt.org/z/JYQb3d

AndrewrJ
  • 49
  • 1
  • 8

2 Answers2

2

The bottom line is that you're calling a non-const method of a constant instance. The (first) coeffRef that is being called is the one (and only) in DenseCoeffsBase.h (DenseCoeffsBase<Derived, WriteAccessors>), which is not const qualified. The DenseCoeffsBase<Derived, ReadOnlyAccessors> class does not have a coeffRef method. You can get around this error (and get a warning) if you enable the -fpermissive compiler flag.

In the dense case, you probably want to use the operator()(Index, Index) method anyway, which does have a const qualified version. I just noticed the documentation explicitly says to use that method anyway, even for the non-const version. This is obviously not going to return a const reference, but at least in your example as a double, it shouldn't matter too much.

Avi Ginsburg
  • 10,323
  • 3
  • 29
  • 56
  • 1
    The reason why I'm asking about ```coeffRef``` is that it's the source of a compilation error when multiplying two ```CwiseUnaryView``` objects. See here for an example: [https://godbolt.org/z/JYQb3d](https://godbolt.org/z/JYQb3d) – AndrewrJ Jun 18 '19 at 07:10
  • 2
    That's a lot messier. I'd wait for ggael to get back to you. I'd also add that to your question, as that's your actual problem (the internal workings of Eigen are making certain assumptions). – Avi Ginsburg Jun 18 '19 at 09:25
  • 1
    Thanks for that, will do – AndrewrJ Jun 18 '19 at 10:31
2

CwiseUnaryView is intended to be used for L-value like expression, e.g.,

MatrixXcd A;
A.real() = something; // `A.real()` is writable

If you want to apply an element-wise functor and use it as an R-value, you should use CwiseUnaryOp instead:

void foo(Eigen::MatrixXd &out)
{
    Eigen::CwiseUnaryOp<dummy_Op, Eigen::MatrixXd> view1(out);
    // shorter:
    auto view2 = out.unaryExpr(dummy_Op());

    Eigen::MatrixXd result = view1 * view2;
    // or directly write: out.unaryExpr(dummy_Op()) * out.unaryExpr(dummy_Op());
}
chtz
  • 17,329
  • 4
  • 26
  • 56
  • 1
    @AndewrJ, What he said ☝. Makes sense, View vs. Op – Avi Ginsburg Jun 19 '19 at 07:12
  • 1
    Is there any way for the method to be agnostic between R-values and L-values? For example: ```A.real() = A.real() * B.real()``` – AndrewrJ Jun 19 '19 at 08:34
  • 1
    @AndrewrJ `A.real()` actually works as R-value as well. I'd need to check where the difference to a custom view is. – chtz Jun 19 '19 at 13:19
  • 1
    @chtz I've narrowed the problem down a bit further. I'm using the ```CWiseUnaryView``` to return a value from a struct that is pointed to by a second struct. The multiplication works fine if the first struct contains a declaration for a primitive type (i.e. int/double) as well as the pointer declaration, but fails without it. See here for an example: [https://godbolt.org/z/7nvhqx](https://godbolt.org/z/7nvhqx) – AndrewrJ Jun 22 '19 at 07:52
  • 1
    It also works with `float` instead of `double`: https://godbolt.org/z/GJUZ41. There seems to be some `sizeof` logic going wrong. Could you report this as a bug to http://eigen.tuxfamily.org/bz/? – chtz Jun 22 '19 at 08:43
  • 1
    I think I've found the problem. ```Eigen/src/Core/CWiseUnaryView.h``` L33: ```int(sizeof(typename traits::Scalar) / sizeof(Scalar))``` Everything compiles after changing this to ```int(sizeof(Scalar))``` – AndrewrJ Jun 22 '19 at 09:57
  • 1
    @chtz is this a safe fix to use for now? I'm just waiting on registration before I can report the bug – AndrewrJ Jun 23 '19 at 08:54
  • 1
    @AndrewrJ I'd have to look into it, but on first glance this rather looks like hiding the problem (and could result in regressions in other cases). – chtz Jun 23 '19 at 17:54