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

Add a broadcasting rotation method #50

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bcbnz
Copy link
Contributor

@bcbnz bcbnz commented Feb 14, 2025

This pull request adds a quat.rotate_broadcast(vecs) method. Standard NumPy broadcasting rules are used to determine the shape of its output. This means that for an Nx4 quaternionic array and an Nx3 vector array, an Nx3 rotated vector array will be output. This is in contrast to the existing .rotate() method which operates as an outer product and would return an NxNx3 rotated vector array.

My initial use case for this is using this for coordinate transforms. If we have a series of poses stored as Nx3 positions and Nx4 quaternions, the result of rotating vectors as part of changing coordinate systems should be Nx3.

Note that I have implemented this with the standard q * v * q.inverse method as some benchmarking indicated this was faster than the Euler-Rodrigues formula given in the docstring of .rotate except for small numbers of rotations. I have uploaded the benchmark script I wrote to https://gist.github.com/bcbnz/72ccebe4cc3d6e5ad3666953bfe3c6d5 and a subset of the generated images are below. The benchmark also asserts the results are allclose with each other and with the existing .rotate() (either fully if the output shapes are the same, or on the first quaternion & vector if not). The benchmarked functions did not include the error checking included in the pull request.

Nx4, 1x3
Nx4, Nx3
Nx10x4, Nx10x3

The unit tests compare results against the .rotate method. In simple cases, these will have the same shape. In the other cases, we can compare with the diagonal vector components.

I have not mentioned the new method in the docstring of .rotate as I wasn't sure what to do with its comments about the alternative method. I am happy to add a commit or do a force push modifying it with some guidance on the appropriate changes, or you are of course welcome to do so yourself (I have set the 'allow edits by maintainers' option in this pull request).

This uses standard NumPy broadcasting rules. This means that for an Nx4
quaternionic array and an Nx3 vector array, an Nx3 rotated vector array
will be output. This is in contrast to the existing rotate() method
which operates as an outer product and would return an NxNx3 rotated
vector array.
Copy link

codecov bot commented Feb 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.92%. Comparing base (cd82c6c) to head (c1c0a01).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   97.90%   97.92%   +0.01%     
==========================================
  Files          10       10              
  Lines        1098     1107       +9     
  Branches      118      120       +2     
==========================================
+ Hits         1075     1084       +9     
  Misses         21       21              
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

2 participants