Drake: Eigen types and extensions used within Drake

Created on 5 May 2016  路  12Comments  路  Source: RobotLocomotion/drake

@sherm1 added:
This issue identifies several Eigen-support utilities we should have access to everywhere in Drake:

  • [ ] standardized aliases for readable abbreviations of Eigen's heavily templatized objects
  • [ ] enabling NaN initialization of otherwise-uninitialized objects in Debug builds
  • [ ] providing convenient NaN initialization for objects that we want _always_ to initialize

@amcastro-tri posted:
Based off the conversation started in #2255, I propose the following solution.
Within a directory named drake/math/Eigen we would place all the Drake standard definitions for Eigen types. This will allow us to do partial specialization on Eigen types allowing us to define very basic and useful objects such as VectorX or Vetor3 only templated on the scalar type (T as per #2255).
Also we could place other useful code in this directory such as Eigen plugings (very useful to define methods not provided by Eigen such as ::Length(), ::SquaredLength() ). Also other particular extensions based on inheritance from Eigen objects could be placed here. I found it useful in the past to have my type <S> class Wrench: public Vector6<S> that would provide additional functionality for wrench objects (Wrench::force(), Wrench::torque() and special operations). Similarly for spatial vectors and twists.
Support for special types (say Poly for instance) could also be placed in this folder as described in Eigen plugings.

To begin with I propose to place in drake/math/Eigen code like so:

// Defined somewhere in drake/common:
template <typename T> using Vector1 = Eigen::Matrix<T, 1, 1>;
template <typename T> using Vector2 = Eigen::Matrix<T, 2, 1>;
template <typename T> using Vector3 = Eigen::Matrix<T, 3, 1>;
// ... maybe up to 6 ...
template <typename T> using Matrix2 = Eigen::Matrix<T, 2, 2>;
template <typename T> using Matrix3 = Eigen::Matrix<T, 3, 3>;
// ... maybe up to 6 ...
template <typename T> using VectorX = Eigen::Matrix<T, Eigen::Dynamic, 1>;
template <typename T> using MatrixX = Eigen::Matrix<T, Eigen::Dynamic, Eigen::Dynamic>;
low dynamics cleanup

Most helpful comment

Per offline chat with sherm1:

  • In Drake debug build(s), set the NAN flag via CMake, not in a header. (CMake offers a way to add variant-specific CXXFLAGS.) This will be less brittle.
  • Promise that we never change Eigen defines in Drake's release builds, and therefore that Drake never depends on having non-default Eigen defines flags set.
  • Consider writing up the NAN-under-debug in the Drake's documentation.

All 12 comments

I agree with this since it will prevent repetitive type definitions scattered throughout the code and standardize the name of the various vectors / matrices. I did this in a previous project here.

Great! good to see other people are doing something similar.

I like this idea. Comments:

  • there is currently no drake/math but we just added drake/common. I think standard abbreviations should go there rather than split off another almost-empty directory. Drake does a _lot_ of math so having a math directory doesn't really narrow things down much.
  • what namespace should these abbreviations be defined in? Eigen supports inserting custom declarations into its classes and namespace, so one possibility would be Eigen::VectorX etc. We could attempt to get those added officially to Eigen next to Eigen::VectorXd etc. Another reasonable alternative would be drake::VectorX.

I also like the idea of standardizing on a SpatialVector alias (inherently a 6-vector) and a few other repeatedly-used aliases like Transform and Quaternion.

Per discussion in #2418, when we invent a centralized place for Eigen goodies we should also include this there:

#ifndef NDEBUG
#  define EIGEN_INITIALIZE_MATRICES_BY_NAN
#endif

That will affect all Eigen objects that have been intentionally left undefined (hopefully for well-justified performance reasons). This shouldn't be used as an excuse not to initialize Eigen objects (to Zero or NaN) when performance permits.

... (proposal to put Eigen settings macros in common header) ...

Wouldn't it be more robust to put that flag in CMakeLists.txt, where it will be in effect 100% of the time (similar to #2181 PRs), instead of a header that might accidentally get forgotten in certain translation units?

That would also have the effect of letting drake-as-library users make their own Eigen decisions, instead of forcing ours on them. (Which may be a bug _or_ a feature, depending on how you look at it.)

Per discussion in simulation-archive@ (Re: Drake and quiet_NaN) on 20160524, we need a standard way to initialize non-performance critical Eigen objects to NaN, that doesn't require several lines of magic incantation. Something on the lines of Vector3d myVec_{NaN}; would be lovely but likely not possible.

Here we are not referring to Debug-only initialization (that can be done with the macro discussed above), but objects where the initialization to NaN is part of the object's contracted behavior always.

Discussion included: @jwnimmer-tri, @sherm1, @amcastro-tri, @liangfok

Wouldn't it be more robust to put EIGEN_INITIALIZE_MATRICES_BY_NAN in CMakeLists.txt, where it will be in effect 100% of the time?

That won't work for a flag that is to be enabled only in Debug builds. For Visual Studio and XCode targets the Release/Debug configuration choice is not made during the CMake run. If we have a header that defines Eigen standard abbreviations & NaN initializers that we use everywhere, I think it will work fine to set this macro in that header. We should then consider it a defect to use Eigen without the eigenhelper header.

Edit: could put this flag in CMAKE_CXX_FLAGS_DEBUG in which case it would work properly.

That would also have the effect of letting drake-as-library users make their own Eigen decisions, instead of forcing ours on them. (Which may be a bug or a feature, depending on how you look at it.)

Bug! BTW, great discussion of why we should never use most of Eigen's collection of behavior-mod macros here in Eigen bugzilla. Luckily the one we're discussing isn't API-altering.

Per offline chat with sherm1:

  • In Drake debug build(s), set the NAN flag via CMake, not in a header. (CMake offers a way to add variant-specific CXXFLAGS.) This will be less brittle.
  • Promise that we never change Eigen defines in Drake's release builds, and therefore that Drake never depends on having non-default Eigen defines flags set.
  • Consider writing up the NAN-under-debug in the Drake's documentation.

OK under one condition: Let's upgrade "Consider writing" in the last bullet of the above comment to "We must write". :smile:

Given that https://github.com/RobotLocomotion/drake/blob/master/drake/common/eigen_types.h exists, I'm not sure there is anything left to do here?

Or rename the issue to be "Use EIGEN_INITIALIZE_MATRICES_BY_NAN in Debug mode" if we still think that's necessary (I don't).

Given that https://github.com/RobotLocomotion/drake/blob/master/drake/common/eigen_types.h exists, I'm not sure there is anything left to do here?

I agree, eigen_types.h addresses most (if not all) of the issues mentioned here. I will therefore close this.

Was this page helpful?
0 / 5 - 0 ratings