metapod defines EIGEN_DEFAULT_TO_ROW_MAJOR

[sbarthelemy]

Metapod #defines EIGEN_DEFAULT_TO_ROW_MAJOR

It is a bad idea, see http://eigen.tuxfamily.org/bz/show_bug.cgi?id=422

Any idea why it was needed?

Was it for performance reasons? Or maybe to have compatibility with another library?

[maxime-reis]

Hi,

It was needed, at first, to be compatible with jrl-dynamics, since the first metapod models were created from jrl-dynamics models. It was easier to convert and compare the models from the two libraries with that macro defined. At least I seem to remember it was the reason, but it goes back to the first test version of metapod, before it was packaged and released. There was something to do with filling matrices with raw data from other matrices (from jrl-dynamics). I am not sure it is still needed, and I'm not even sure removing it would cause any difference in behaviour. The non-regression tests should help with the potential debugging though.

Maxime REIS

On 30 October 2012 10:52, Sébastien BARTHÉLÉMY notifications@github.comwrote:

Metapod #defines EIGEN_DEFAULT_TO_ROW_MAJOR

It is a bad idea, see http://eigen.tuxfamily.org/bz/show_bug.cgi?id=422

Any idea why it was needed?

Was it for performance reasons? Or maybe to have compatibility with another library?

— Reply to this email directly or view it on GitHubhttps://github.com/laas/metapod/issues/15.

[sbarthelemy]

Hello Maxime,

thank you for providing me with the background. I'll remove the macro and check.

[sbarthelemy]

The jac_point perf regression is annoying but I do not have the time to look at it immediately.

I can imagine several ways to "fix" it:

• update the algorithm to recover the previous performance (not sure this is possible)
• use Eigen::RowMajor in the algorithm to get RowMajor storage order only in the matrices used by this algorithm
• use Eigen::RowMajor in metapod-wide Eigen typedef to get the previous behaviour back, but without the problems

[aelkhour]

I discovered the origin of the jac_point performance regression:

The jac_point algorithm computes the 6xRobot::NBDOF articular jacobian of a specific body. In order to test jac_point for all robot bodies, I implemented jac_point_robot which calls jac_point Robot::NBBODIES times and computes the 6*Robot::NBBODIESxRobot::NBDOF jacobian. jac_point_robot is called too in the benchmark.

While jac_point fills recursively the body jacobian with 6D column vectors for revolute joints (which would make the algorithm faster when the #define is removed), jac_point_robot fills recursively the robot jacobian with 6*Robot::NBDOF block matrices. After running profiling tests, it turns this operation takes up 67% of the computation time when #define is removed, compared to 30% with #define.

As jac_point_robot is used only for testing purposes, this is not a major issue. I will nevertheless try to use Eigen::RowMajor and see if this solves the problem.

[sbarthelemy]

ok, I'll push within minutes a small cleanup commit that factors out the confVector definition. if you play with Eigen::RowMajor that might help (not sure what it means for a vector thought).

[aelkhour]

I only set the robot jacobian type in jac_point_robot to Eigen::RowMajor and it did the trick. Now I have roughly the same performance for jac_point_robot as before.