-
Notifications
You must be signed in to change notification settings - Fork 789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Eigen Serialization Fix #944
Conversation
Is there another way to side-step the issue in Link? |
Updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not following. I guess this PR is no longer an Eigen upgrade, but a workaround for serialization? Note this was not what I advised in the associated issue.
My understanding was that you preferred the workaround, so I went with that approach. Eigen is no longer touched (we're still at good ol' 3.3.7) but it resolves the serialization issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved after Ci succeeds
Eigen has backported some fixes which enable correct serialization when using GCC 8 and above.
The issue is detailed here:
https://gitlab.com/libeigen/eigen/-/issues/1676
Basically, boost serialization with Eigen throws an error because of incorrect template deduction handling by GCC. This issue is not seen in Clang, but the Eigen team went ahead and made a simple fix.
The only issue left to resolve is what happens when a user uses the system Eigen rather than the packaged version under
3rdparty
. One potential solution is to specify the required Eigen version in CMake (cmake/HandleEigen.cmake
). However, I wanted more discussion on this before pushing that change.An example of the error thrown is when we try to serialize the
Link
object in GTDynamics, which has a Vector3 for the inertial matrix diagonal: