Skip to content
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

Pose3 interpolateRt method #647

Merged
merged 6 commits into from
Dec 31, 2020
Merged

Pose3 interpolateRt method #647

merged 6 commits into from
Dec 31, 2020

Conversation

varunagrawal
Copy link
Collaborator

@varunagrawal varunagrawal commented Dec 31, 2020

New interpolateRt method that might be more intuitive than minimizing geodesic distance.

@varunagrawal varunagrawal added bugfix Fixes an issue or bug high-priority Need this done quickly labels Dec 31, 2020
@varunagrawal varunagrawal self-assigned this Dec 31, 2020
Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the old code is correct. The interpolate function is defined as doing interpolation in tangent space. If the rotations of both endpoints are different, there is no expectation that the translation linearly interpolates.

If the rotations are the same, I think it does linearly interpolate translation (but I'm not even sure of that).

@varunagrawal
Copy link
Collaborator Author

Actually, the old code is correct. The interpolate function is defined as doing interpolation in tangent space. If the rotations of both endpoints are different, there is no expectation that the translation linearly interpolates.

If the rotations are the same, I think it does linearly interpolate translation (but I'm not even sure of that).

I though about that at first, that perhaps the interpolation of the translations needn't be linear betweem them if the rotations are different, but after looking it up and checking the implementation of other libraries, it seems that the translations are indeed linear even if the rotations are different.

This is one of the reasons I put that example from Peter Corke's video lecture as a unit test. It is based off his Robotics Toolbox for Matlab.

// This interpolation is easy to calculate by hand.
double t = 0.5;
Pose3 expected0(Rot3::Rz(M_PI_4), Point3(0.5, 0, 0));
EXPECT(assert_equal(expected0, interpolate(start, end, t)));
Copy link
Collaborator Author

@varunagrawal varunagrawal Dec 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dellaert I added this simple test to verify since it is easy to visualize and imagine: a body turning about Z as it moves from the origin to (1, 0, 0).

The old code fails with this message:

Not equal:
expected:
 R: [
	0.707107, -0.707107, 0;
	0.707107, 0.707107, 0;
	0, 0, 1
]
t: 0.5   0   0
actual:
 R: [
	0.707107, -0.707107, 0;
	0.707107, 0.707107, 0;
	0, 0, 1
]
t:       0.5 -0.207107         0

It doesn't make any sense why the body would off the X axis.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because the interpolation is in tangent space. Straight lines in tangent space correspond to screw motions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please provide a reference I can read up on?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://en.wikipedia.org/wiki/Slerp

The reference is the documentation of "interpolate". I think you expected something else, which is fine. I emailed with a different approach (interpolateRt). You can document the difference with interpolate there.

Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Please document the difference with interpolate in the doxygen? And I'd also change the test to use interpolateRt now, right? interpolate should be tested in testRot3, and interpolate should be tested in testPoint3.

@varunagrawal
Copy link
Collaborator Author

I see. By shortest distance I meant straight line. Will update.

Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's perfect :-)

@varunagrawal varunagrawal changed the title Pose3 template specialization for interpolate Pose3 interpolateRt method Dec 31, 2020
@varunagrawal varunagrawal merged commit bac74db into develop Dec 31, 2020
@varunagrawal varunagrawal deleted the fix/pose3-interpolate branch December 31, 2020 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes an issue or bug high-priority Need this done quickly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants