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

Multiplication with scalar depend on order #18

Closed
hovren opened this issue Aug 4, 2015 · 3 comments
Closed

Multiplication with scalar depend on order #18

hovren opened this issue Aug 4, 2015 · 3 comments

Comments

@hovren
Copy link

hovren commented Aug 4, 2015

Order of multiplication of scalar and quaternion matters. It works fine from right:

>>> q = np.quaternion(1,2,3,4)
>>> q * 5
quaternion(5, 10, 15, 20)

But not from left:

>>> 5 * q
TypeError: Input object is not a quaternion.

Even worse is that if you do this with an array of quaternions, there is no warning, but the result seems to be more or less random.

>>> q * np.arange(5)  # OK!
array([quaternion(0, 0, 0, 0), quaternion(1, 2, 3, 4),
   quaternion(2, 4, 6, 8), quaternion(3, 6, 9, 12),
   quaternion(4, 8, 12, 16)], dtype=quaternion)

>>> np.arange(5) * q  # Woops!
array([quaternion(4, 8, 12, 16), quaternion(0, 0, 0, 0),
   quaternion(0, 0, 0, 0), quaternion(0, 0, 0, 0),
   quaternion(0, 0, 0, 0)], dtype=quaternion)
@moble
Copy link
Owner

moble commented Aug 4, 2015

Thanks for opening this issue. I've figured out why 5*q wasn't working and found a bunch of less common bugs caused by the same line of code. I've added tests for this and the various other problems I turned up, and made sure they all pass. Will push that momentarily.

I'll have to dig a little more to figure out the array problem, but I'm working on it.

moble added a commit that referenced this issue Aug 4, 2015
Issue #18 noted that `5*q` didn't work.  I traced this to the altogether terrible line 268 in
numpy_quaternion.c.  Also found additional errors that this line caused:
  * `5*q` didn't work
  * 5.0/q was converted to q/5.0
  * 5.0**q was converted to q**5.0

To make this easier to deal with, I added some scalar multiplication, etc., functions to quaternion.h.

Also made powers like 1**0, 0**0, etc., consistent with python's behavior.

Made sure that all combinations of powers where the base was scalar and exponent was quaternion, etc., work.

Added tests for all problems I found.
moble added a commit that referenced this issue Aug 5, 2015
I think the problem with arrays pointed out in issue #18 resulted from numpy dropping down
to functions I didn't expect it to use -- thus enabling functionality I didn't intend to support.
My attempt to fix it involves adding the functions explicitly.  I hope this is all I need...

This basically resolves the issue noted above, but I want to add all the tests I can think of, to
make sure nothing like this is still slipping through.

Also some formatting changes
@moble moble closed this as completed in c051d58 Aug 5, 2015
@moble
Copy link
Owner

moble commented Aug 5, 2015

I've added lots of tests to check all the (binary) ufuncs. So I'm satisfied that the second problem you pointed out is now also solved.

Thanks again for pointing this out, and please let me know if you find any more problems.

@hovren
Copy link
Author

hovren commented Aug 6, 2015

I can confirm that it works now. Thanks!

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

No branches or pull requests

2 participants