Skip to content

Fix tolerance for axis-angle conversion of quaternions#620

Merged
scpeters merged 4 commits intogazebosim:gz-math7from
shameekganguly:quaternion_aa
Aug 23, 2024
Merged

Fix tolerance for axis-angle conversion of quaternions#620
scpeters merged 4 commits intogazebosim:gz-math7from
shameekganguly:quaternion_aa

Conversation

@shameekganguly
Copy link
Copy Markdown
Contributor

🦟 Bug fix

Summary

Quaternions smaller than 1e-3 in length are currently treated as 0 when converting to axis angle. This is because the squared length of the quaternion is compared to 0 with an epsilon of 1e-6. The fix is to compare the length of the quaternion instead of the squared length so that only a quaternion of length smaller than 1e-6 (default epsilon in math::equal(...)) is treated as zero for this conversion.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@github-actions github-actions bot added 🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic labels Aug 22, 2024
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@azeey
Copy link
Copy Markdown
Contributor

azeey commented Aug 22, 2024

There's a new compiler warning on macOS. The previous build (https://build.osrfoundation.org/job/gz_math-ci-pr_any-homebrew-amd64/170/) didn't have this. Not sure if that's because the OS versions are different or doing the sqrt as before avoids the warning.

Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@shameekganguly
Copy link
Copy Markdown
Contributor Author

There's a new compiler warning on macOS. The previous build (https://build.osrfoundation.org/job/gz_math-ci-pr_any-homebrew-amd64/170/) didn't have this. Not sure if that's because the OS versions are different or doing the sqrt as before avoids the warning.

Looks like we have a template instantiation for Quaternion with integer data in https://github.com/gazebosim/gz-math/blob/gz-math7/src/python_pybind11/src/Quaternion.hh which caused this check to trip.
Added static_cast to T in a406769

@scpeters scpeters changed the title Fix issue where small quaternions are treated as zero for conversion to axis angle Fix tolerance for axis-angle conversion of quaternions Aug 23, 2024
@scpeters scpeters merged commit b0989bb into gazebosim:gz-math7 Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants