Skip to content

Changes the quat_box_minus implementation and adds quat_box_plus and rigid_body_twist_transform #2217

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jtigue-bdai
Copy link
Collaborator

@jtigue-bdai jtigue-bdai commented Apr 1, 2025

Description

  • Changes the quat_box_minus implementation see changes for reference links
  • Adds quat_box_plus and rigid_body_twist_transform methods

Reason for Change

While using quat_box_minus to get angular velocities through finite differences we encountered that the angular velocity contained some unreasonably large values at given points along the trajectory. After some investigation we observed:

  • The problem comes from quat_box_minus yielding unreasonably large axis-angle differences at some points (”outliers”) of the trajectory.
  • Those “outliers” appear when the real part of the difference quaternion (quat_diff = quat_mul(q1, quat_conjugate(q2))) becomes negative ($w < 0$).
  • This happens even when the inputs are standardized with $w≥0$.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

Copy link
Collaborator

@jsmith-bdai jsmith-bdai left a comment

Choose a reason for hiding this comment

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

LGTM! Also, thanks for adding tests

Comment on lines +540 to +541
quat_diff = quat_mul(q1, quat_conjugate(q2)) # q1 * q2^-1
return axis_angle_from_quat(quat_diff) # log(qd)
Copy link
Contributor

@Mayankm96 Mayankm96 Apr 25, 2025

Choose a reason for hiding this comment

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

Hmm was the previous implementation wrong? I remember checking that the above doesn't work well for small angles so made a better implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey @Mayankm96 I added some explanation to the PR description and on the removed portions of the code. Let me know if you have any questions.

def rigid_body_twist_transform(
v0: torch.Tensor, w0: torch.Tensor, t01: torch.Tensor, q01: torch.Tensor
) -> tuple[torch.Tensor, torch.Tensor]:
r"""Transform the linear and angular velocity of a rigid body between reference frames.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might need to fix the docstring enters to make sure it is rendered properly.

Comment on lines -503 to -518
def quat_box_minus(q1: torch.Tensor, q2: torch.Tensor) -> torch.Tensor:
"""The box-minus operator (quaternion difference) between two quaternions.

Args:
q1: The first quaternion in (w, x, y, z). Shape is (N, 4).
q2: The second quaternion in (w, x, y, z). Shape is (N, 4).

Returns:
The difference between the two quaternions. Shape is (N, 3).
"""
quat_diff = quat_mul(q1, quat_conjugate(q2)) # q1 * q2^-1
re = quat_diff[:, 0] # real part, q = [w, x, y, z] = [re, im]
im = quat_diff[:, 1:] # imaginary part
norm_im = torch.norm(im, dim=1)
scale = 2.0 * torch.where(norm_im > 1.0e-7, torch.atan2(norm_im, re) / norm_im, torch.sign(re))
return scale.unsqueeze(-1) * im
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • When atan2 is being used in this function, it doesn’t account for the sign of re . Notice how the approximation takes the sign of re to compute the scale value when norm_im is sufficiently small but doesn’t take that into account for the non-approximated value.

  • This causes that whenever there is a negative real part of the quaternion difference re , the scale will overestimate the rotation and will take the longest path between orientations instead of the shortest. E.g. whenever we go from -0.1 rad to 0.1 rad in a single axis this will compute a -(2pi-0.2) rad rotation instead of a 0.2 rad rotation.

  • This will happen whenever re = quat_diff[:,0] is negative.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

3 participants