-
-
Notifications
You must be signed in to change notification settings - Fork 942
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
Quaternion multiplication wrong, because of Tensor.cross #2822
Labels
help wanted
Extra attention is needed
Comments
Good catch, please go ahead with the PR and it would be great if you can update the tests as well! |
7 tasks
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
The expression
c
should give the same result but it actually goes wrong.The code gives
May affect more behaviors
For example the so3 and se3 lie group actions also depend on quaternions.
Tracing the bug
The bug is caused by a call to
Tensor.cross
, which finds the first dimension of size=3 as vec3. The code can be found inQuaternion.__mul__
. Replacing the call with a more stabletorch.linalg.cross
can fix the issue, according to my experiment.It is weird that only expression
c
actually triggers the bug. PyTorch actually gives a warning of Tensor.cross deprecated but no one seems to be caring?Can I possibly help fix it in a PR?
One more question
Is Quaternion class designed to be not supporting broadcasting? Why those modules default to be
requires_grad=True
even when initializers arerequires_grad=False
?Reproduction steps
Expected behavior
All four expressions should give the same result.
Environment
Additional context
No response
The text was updated successfully, but these errors were encountered: