Skip to content
This repository has been archived by the owner on Nov 29, 2019. It is now read-only.

use Eigen abstract types instead of concrete types when possible #62

Open
thomas-moulard opened this issue Apr 18, 2013 · 7 comments
Open

Comments

@thomas-moulard
Copy link
Contributor

Dear all,
I have the following code which is sub-optimal:

  robot_t::confVector torques;
  for (unsigned frameId = 1; frameId < animatedMesh_->numFrames () - 1;
   ++frameId)
{
  metapod::rnea<robot_t, true>::run
    (robot, q[frameId], dq[frameId], ddq[frameId]);
  metapod::getTorques (robot, torques);

  result.segment
    (frameId * robot_t::NBDOF, robot_t::NBDOF) = torques;
}

...the following version would avoid a copy but is not possible right now:

  metapod::getTorques (robot, result.segment
    (frameId * robot_t::NBDOF, robot_t::NBDOF));

Would it be possible to make sure that algorithms and functions take a EigenBase<T>, MatrixBase<T> or any other adequate abstract type instead of concrete type so that metapod can avoid enforcing its own matrix type confVector to everyone.

A side effect would be to let the user choose its own scalar type, matrix implementation, etc. which could lead to many potential optimization on the end-user side.

Thanks!

@olivier-stasse
Copy link
Member

On 04/18/2013 07:16 AM, Thomas Moulard wrote:

Dear all,
I have the following code which is sub-optimal:

| robot_t::confVector torques;
for (unsigned frameId = 1; frameId < animatedMesh_->numFrames () - 1;
++frameId)
{
metapod::rnea<robot_t, true>::run
(robot, q[frameId], dq[frameId], ddq[frameId]);
metapod::getTorques (robot, torques);

result.segment
(frameId * robot_t::NBDOF, robot_t::NBDOF) = torques;
}
|

...the following version would avoid a copy but is not possible right now:

| metapod::getTorques (robot, result.segment
(frameId * robot_t::NBDOF, robot_t::NBDOF));
|

Would it be possible to make sure that algorithms and functions take a
|EigenBase|, |MatrixBase| or any other adequate abstract type
instead of concrete type so that metapod can avoid enforcing its own
matrix type |confVector| to everyone.

I agree that this is a problem.

A side effect would be to let the user choose its own scalar type,
matrix implementation, etc. which could lead to many potential
optimization on the end-user side.

This is already a feature in the pile of TODO list for the scalar type.

Regarding the matrix implementation we could but I do not think it is a
good idea.
The Eigen matrix unlike a lot of other matrix implementation, performs
some kind of
formal computing which I believe to be quite important in the current
performances.
I currently have a bad experience with trying to make a generic matrix
implementation
(jrl-*).

More generally, the way the data are stored is still not fixed due to
current tests
of metapod on two differents architectures:

  • One lead by Sebastien Barthelemy for him and/or Aldebaran (code) on Nao ?
  • One lead by me for porting abstract-robot-dynamics.

This may lead to dramatic changes between releases.

Finally due to other commitments, my current involvment in metapod
is unfortunately limited...

However the confVector issue should be not too difficult to fix, I'll
try to propose a fix.

Thanks!


Reply to this email directly or view it on GitHub
#62.

@thomas-moulard
Copy link
Contributor Author

Sorry, I was not clear enough.

You currently have:

myalgo(confVector&)

You should replace it by:

 template <typename T>
 myalgo(Eigen::MatrixBase<T>&)

...this would not impact the performances as the type will be resolved at compile time.
In the worst case, you still can template by T and add a static assert making sure that T inherits
from MatrixBase.

@sbarthelemy
Copy link

@thomas-moulard:

regarding the scalar type, the current plan is to template all the Spatial classes and the models ones by the Scalar type, à la Eigen.

Yes the current code involves copies and conversion to concrete Eigen types, which should be avoided. An alternative to your suggestion would be to use the new "Ref" classes that Eigen 3.2 (not released yet) introduces.
I'd like to give it a try, and in any way fix these conversions, but have no ETA yet.

@thomas-moulard
Copy link
Contributor Author

Yes, unfortunately, Ref are very new and I had some trouble with block operations with them. Also relying on bleeding edge version makes metapod a bit harder to use. But I agree with you, in a couple of months, ref may be the solution to a lot of problems :)

@olivier-stasse
Copy link
Member

Dear all,
I just proposed a version of metapod with a templated scalar type in topic/float-type-templated.
This do not change too much the robot API. The benchmarks, robotbuilder and sample template have been update accordingly all the unitary test runs, and there is no impact on the performance as expected.
Best,
Olivier

@olivier-stasse
Copy link
Member

I would like some comments/review before merging to the master (soon).

@thomas-moulard
Copy link
Contributor Author

FYI Eigen 3.2.0 contains Ref. They allow to pass matrices independently of their real type (block, etc.) without requiring the code to be templated. It would be perfect to implement this feature with Ref(s).

Sorry I don't have time to do that myself though.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants