gazebosim / gz-math

General purpose math library for robot applications.
https://gazebosim.org/libs/math
Apache License 2.0
52 stars 62 forks source link

Consider using eigen #19

Open osrf-migration opened 9 years ago

osrf-migration commented 9 years ago

Original report (archived issue) by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Eigen provides Vector, Matrix, Quaternion, and Affine transform (pose) types. We should consider using it. (Migrated from gazebo issue 929).

It looks like it is portable. One major thing we are missing is the ability to use arbitrary matrices (necessary for Jacobians). Eigen supports arbitrarily-sized matrices, as well as sparse matrices.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


We can also consider Blaze:

osrf-migration commented 8 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


There is some discussion on the fcl issue tracker about using eigen:

osrf-migration commented 7 years ago

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


I'm closing the analogous Gazebo issue now that Gazebo math has been fully deprecated in favor of ignition math. Please consider the discussions on that issue as well, there are a lot of nice comments there:

https://bitbucket.org/osrf/gazebo/issues/929/consider-using-eigen-to-improve-math

osrf-migration commented 6 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


cc @mxgrey

osrf-migration commented 6 years ago

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).


I definitely think using Eigen would add a lot of value to ignition::math, and simultaneously ignition::math would add a lot of value to Eigen. This would likely result in a much broader adoption and larger community for ignition::math, even going beyond the robotics community.

The tricky part is that Eigen structures are not easy to wrap or extend due to the implementation details of their templates. We would need to be very smart about how we interface with Eigen and how we approach incorporating it into the larger goals of ignition. I personally believe the added difficulty would be very much worthwhile in the long run, but I think there are reasonable arguments to be made against it.

osrf-migration commented 6 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Maybe we could start with a companion package called ignition-math-eigen that provides conversion functions back and forth between ignition and eigen types. For now, this would keep ign-math free of an eigen dependency. Since DART uses eigen, it wouldn't be a big leap to have gazebo depend on ignition-math-eigen if DART is found.

osrf-migration commented 6 years ago

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).


Helper functions would definitely be good if we decide not to use Eigen natively. I'm somewhat of the opinion that, if we're going to change the underlying math library, then we should make that decision as soon as possible to minimize the overhead of making that change (but of course the most important thing is to make a good decision rather than a hasty one).

osrf-migration commented 6 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Switching to Eigen is a big change, so I thought a baby step with ignition-math-eigen would give us a taste of the packaging difficulties before jumping in all the way. We could consider it part of the decision-making process.

osrf-migration commented 6 years ago

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).


Honestly, the packaging is completely trivial compared to the challenge of making it the native basis data structure. We could even just pull the Eigen headers into the ign-math repo if we wanted to make it literally 100% trivial, although I'd strongly prefer making it an external dependency, because that users can benefit from the latest revisions without us needing to keep our copy of Eigen's source tree up to date.

osrf-migration commented 6 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I agree that changing the native basis data structure is a huge challenge, but that doesn't mean we should ignore packaging, since I expect that will be a source of concern when adding a dependency to a package that currently doesn't have any. By packaging, I also include updating documentation of installation instructions and CI scripts. I mean, I think it's worth doing anyway even if we don't make the big switch, and getting our feet wet with packaging will be work we'd have to do anyway if we make the big switch.

osrf-migration commented 6 years ago

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).


I've noticed that there are some (almost certainly unintentional) inconsistencies in the APIs provided by Vector2, Vector3, and Vector4 as alluded to in pull request #181. This kind of thing naturally occurs in code with a WET anti-pattern.

I think the ideal solution to this is to implement these classes using template arguments for the number of components, similar to the way Eigen does. The thing is, at that point we'll be on the path to reimplementing a less-developed version of Eigen, so we'd be better off just using Eigen wholesale.

osrf-migration commented 6 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I still think that conversion functions in a separate ignition-math-eigen package would be a good first step.

osrf-migration commented 5 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


The use of eigen is not on our roadmap right now. This might change in the future, but for now there are more important features and issues to tackle.

osrf-migration commented 5 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


@nkoenig eigen would allow a nice way to handle 2d simulation with similar data structures. Is it possible to keep eigen as a build dependency for ign-physics but only use ignition-math types in the API that faces ign-gazebo?

osrf-migration commented 5 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


Here is some more explanation about my concerns.

I get the value proposition of eigen, but I need to weigh it against the implementation cost and our obligations. Right now the cost is high relative to the value. Gazebo has worked for 16 years without eigen, no one is banging at our door for 2d simulation, and we have high risk features to implement.

We will get to eigen integration, but it can't happen right now. Importantly, I don't want to be in the situation where we have inconsistencies between the ignition libraries. I'd like the ignition libraries to be curated by us, and reflect a consistent and purposeful design.

We can revisit eigen after we are in good place with gazebo11's core features. This will likely be sometime middle of next year.

We can leave eigen as a build dependency.

osrf-migration commented 5 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


I should note that you are more than welcome to use eigen if it doesn't effect a public API.