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

Quaternion rotation of vector modifies object instance #55

Closed gibsjose closed 3 years ago

gibsjose commented 4 years ago

I'm using pyquaternion to generate unit tests for some C code which performs various quaternion and vector operations such as the quaternion Hamilton product, rotation a vector with a quaternion, etc.

I noticed that the results from my C code was not matching the pyquaternion result, and then I noticed that when using basic numpy arrays I get matching answers, but not when using the Quaternion class.

Digging further into it, I found that it was related to the fact that I was calling q.rotate(v) to rotate a vector with a quaternion, and that the rotate method is normalizing q and that modifies the object thereafter, so methods later in my code that used the same quaternion were working with a different quaternion.

This short snippet shows the behaviour:

from pyquaternion.quaternion import Quaternion
import numpy as np

a = Quaternion(0.0875, -0.8330, 0.2902, -0.4629)
b = Quaternion(-0.4156, 0.8189, -0.3817, 0.1043)
v = [1.0000, 1.0000, 1.0000]

print([np.float32(i) for i in a * b])
a.rotate(v)
print([np.float32(i) for i in a * b])

Prints

[0.8048285, 0.27142748, -0.44619277, 0.2818188]
[0.8048133, 0.27142236, -0.44618437, 0.2818135]

The first answer (before the quaternion was normalized) matches my C code and also WolframAlpha.

While I think the case could be made either way, I think the least surprising thing for the user would be to create a temporary quaternion object from the parameter, normalize that, and return the result of the rotation. This way the input quaternion remains the same and the result is unchanged.

alphaville commented 3 years ago

I suspect this must be due to some normalisation step. For example, if you run the following code:

import numpy as np
from pyquaternion import Quaternion

a = Quaternion(0.0875, -0.8330, 0.2902, -0.4629)
b = Quaternion(-0.4156, 0.8189, -0.3817, 0.1043)
v = [1.0000, 1.0000, 1.0000]

a = a.normalised
b = b.normalised
print([np.float32(i) for i in a * b])
for i in range(100):
    a.rotate(v)
print([np.float32(i) for i in a * b])

You will get:

[0.804856, 0.27143675, -0.44620803, 0.28182843]
[0.804856, 0.27143675, -0.44620803, 0.28182843]
KieranWynn commented 3 years ago

Yes, there is an implicit normalisation. This is resolved in https://github.com/KieranWynn/pyquaternion/pull/47

On Mon, 5 Oct 2020 at 9:21 am, Pantelis Sopasakis notifications@github.com wrote:

I suspect this must be due to some normalisation step. For example, if you run the following code:

import numpy as np

from pyquaternion import Quaternion

a = Quaternion(0.0875, -0.8330, 0.2902, -0.4629)

b = Quaternion(-0.4156, 0.8189, -0.3817, 0.1043)

v = [1.0000, 1.0000, 1.0000]

a = a.normalised

b = b.normalised

print([np.float32(i) for i in a * b])

for i in range(100):

a.rotate(v)

print([np.float32(i) for i in a * b])

You will get:

[0.804856, 0.27143675, -0.44620803, 0.28182843]

[0.804856, 0.27143675, -0.44620803, 0.28182843]

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/KieranWynn/pyquaternion/issues/55#issuecomment-703324283, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNB5GW7EXR65LKORXM2XJ3SJDYPFANCNFSM4KT5KSYQ .

alphaville commented 3 years ago

@KieranWynn Is #47 expected to be merged?

KieranWynn commented 3 years ago

It's a breaking change, so will be part of 1.0 release. Can't give you a timeline on this just yet, unfortunately.