Registered Member
|
For writing interfaces that may access portions of contiguous Eigen matrices, may I ask if we might be shooting ourselves in the foot if we permitted access via `Ref<CompatibleT>` rather than `Block<ExactT>` (both in run-time and in memory management)?
It'd be nice to hide the implementation details if possible (e.g., if the storage is fixed- or dynamic-size, or if we are using an `Eigen::Map<>` under the hood for library interop), which it seems like `Ref<>` would permit, but `Block<>` would be quite persnickety about. Looking at the implementation of `Ref<>` and `RefBase<>`, it looks like it extends `MapBase<>`, which I believe implies that it stores the data pointer and structure (size, stride, etc.) and does not depend directly on the object it was constructed from. Specifically, we've done a quick test to ensure that we could construct a `Ref<>` from a temporary object (`VectorBlock<>`):
Pushing this through valgrind memcheck, ASan, MSan, UBSan, etc., doesn't seem to show any foul behavior. Background post, with source code for doing the testing: https://github.com/RobotLocomotion/drake/issues/6969 |
Moderator
|
Ref<> was rather designed for function parameters rather than as return type. Here, with c++14, you could just return auto unless the details of the implementation is hidden in a .cpp file. If your functions or local variables use Ref<VectorXd> then it's still fine to return auto aka Block<> in this case. Nonetheless, it is still perfectly fine to return a non-const Ref<> as in your case because if you pass a non compatible type, like MatrixXd::row(i) which is non contiguous, then Ref<VectorXd> won't accept it and will trigger a compile-time error. On the other hand, it would dangerous to return a Ref<const VectorXd> because a temporary might be created if you construct it from a non-compatible type. See for instance: http://eigen.tuxfamily.org/bz/show_bug.cgi?id=884
|
Registered Member
|
Thank you for the detailed response!
For the API of my stuff, the implementation is presently located in a separate source file, and I'd prefer to encapsulate it at the compile-time level, if possible, not just via the user API (and would still like it to be constrained to Eigen types). From looking at the bug report, it seems like one workaround to avoid the temporaries would be to define a new reference class which more or less leverages the existing `Ref` + `Map` implementation, but eliminating the `Ref<const Derived>` specialization (effectively offering the `NoCopy` option). I've drawn up a small prototype here: https://github.com/EricCousineau-TRI/re ... opy.cc#L51 https://github.com/EricCousineau-TRI/re ... output.txt I had originally tried using `MapBase<>` directly, but found it to be a little kludgy when dealing with specialized outer strides (accessing `.block()`s), whereas `RefBase<>` seemed to handle those edge cases. Does this workaround (a) make your blood curdle or (b) seem sufficient for having non-implicit-copy references? |
Moderator
|
Registered users: Bing [Bot], daret, Google [Bot], sandyvee, Sogou [Bot]