Skip to content

Conversation

@pschoc
Copy link

@pschoc pschoc commented Nov 7, 2025

Description

Related Issue

Resolves Genesis-Embodied-AI/Genesis#

Motivation and Context

How Has This Been / Can This Be Tested?

Screenshots (if appropriate):

Checklist:

  • I read the CONTRIBUTING document.
  • I followed the Submitting Code Changes section of CONTRIBUTING document.
  • I tagged the title correctly (including BUG FIX/FEATURE/MISC/BREAKING)
  • I updated the documentation accordingly or no change is needed.
  • I tested my changes and added instructions on how to test it for reviewers.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@pschoc
Copy link
Author

pschoc commented Nov 7, 2025

also I have trouble understanding the skew:

how is the "skew" meant to be used or what is it expressing? From what I see in imu.py/_get_skew_to_alignment_matrix() it generates a rotation matrix of the form:
[ 1 v1 v2 ]
[ v0 1 v2 ]
[ v0 v1 1 ]
given a skew vector [v0, v1, v2]. That makes no sense to me, I was expecting sth like eye(3) + hat([v0,v1,v2]) =
[ 1 -v2 v1 ]
[ v2 1 -v0 ]
[ -v1 v0 1 ]
or handing over the vector of angular misalignments which then get mapped from euler angles to so3

@duburcqa
Copy link
Collaborator

duburcqa commented Nov 7, 2025

@Milotrince Why the unit test is passing ?

@Milotrince
Copy link
Contributor

@pschoc Thanks for the catch.

@duburcqa I will update the unit test to cover this, i think the current test is too simple (local orientation = global orientation) so it didn't catch it.

also I have trouble understanding the skew:

The axis skew is supposed to represent sensor axis misalignment / cross-axis coupling.

matlab also uses this definition, for reference: https://www.mathworks.com/help/nav/ref/imu.html
Screenshot 2025-11-07 at 10 10 22

@Milotrince
Copy link
Contributor

maybe we could rename axes_skew to axes_misalignment or axes_coupling to avoid confusion ?

@Milotrince Milotrince mentioned this pull request Nov 9, 2025
5 tasks
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