KieranWynn / pyquaternion

A fully featured, pythonic library for representing and using quaternions
http://kieranwynn.github.io/pyquaternion/
MIT License
339 stars 68 forks source link

Unexpected normalization in rotation functions #39

Closed jkunimune closed 4 years ago

jkunimune commented 5 years ago

There is an undocumented implicit normalization in the functions Quaternion.rotate() and Quaternion.rotation_matrix(), which should be either documented or removed. I would lean toward the latter. If a user tries to rotate with a quaternion that is not unit, then the expected behaviour would be to either use that quaternion's normalized form without changing the actual object, or to raise an error.

>>> q = Quaternion([1, 0, 1, 0])
>>> q.magnitude
1.4142135623730951
>>> q.rotate([1, 0, 0])
[0.0, 0.0, -0.9999999999999998]
>>> q.magnitude # expected: 1.4142135623730951
0.9999999999999999

As an example of a situation where it would make sense to remove the implicit normalization, I'm making a simulation that uses Quaternions to represent orientation. My Quaternion magnitude naturally drifts over time due to the imperfection of ODE solvers, so I tried to add a normalizing restoring force. However, I found that my Quaternion (whose rotation matrix I took a few lines before) was already normalized, even though the state vector I had derived it from clearly was not.

KieranWynn commented 4 years ago

Hi jkunimune15 Thanks for pointing this out. You are correct - the implicit behaviour here is undesirable - documented or not. I'm going to go further than your existing PR and remove ALL implicit normalisations and instead do as you recommend - use a normalised copy of the quaternion object (if required) to perform the operation. I will be sure to update the docs to reflect this.

Thanks for your input

K

KieranWynn commented 4 years ago

Fixed in https://github.com/KieranWynn/pyquaternion/pull/47