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

Memory leak when assigning a numpy array to .vec atribute? #25

Closed
jsbtic opened this issue Aug 23, 2016 · 3 comments
Closed

Memory leak when assigning a numpy array to .vec atribute? #25

jsbtic opened this issue Aug 23, 2016 · 3 comments

Comments

@jsbtic
Copy link

jsbtic commented Aug 23, 2016

Dear all,
I ran into some memory problems when changing the values of a quaternion object's .vec -attribute. This becomes exceedingly problematic in for loops, where memory consumptions increases steadily after every iteration step.

C.f. using memory_profiler package, v. 0.41 ( conda install -c anaconda memory_profiler )

$ mprof run quaternion_minimal_example.py 
mprof: Sampling memory every 0.1s
running as a Python program...
Filename: quaternion_minimal_example.py

Line #    Mem usage    Increment   Line Contents
================================================
    10     66.8 MiB      0.0 MiB   @profile
    11                             def main():
    12     66.8 MiB      0.0 MiB       N = int(1e6)
    13     66.8 MiB      0.0 MiB       q = np.quaternion(0, 0, 0, 0)
    14                             
    15    136.5 MiB     69.8 MiB       for n in xrange(N):
    16    136.5 MiB      0.0 MiB           q.vec = np.array([0, 0, 0])

(use mprof plot to show the memory consumption trend.)

Here's the code for quaternion_minimal_example.py

import numpy as np
import quaternion
from memory_profiler import profile


@profile
def main():
    N = int(1e5)
    q = np.quaternion(0, 0, 0, 0)

    for n in xrange(N):
        q.vec = np.array([0, 0, 0])


main()

The problem can be solved if one replaces np.array([0, 0, 0]) with np.array([0, 0, 0]).tolist(). Is there perhaps a bug in the quaternion package, or am I just doing something wrong...(?)

My best,

  • J.
@moble
Copy link
Owner

moble commented Aug 24, 2016

Thanks for the bug report!

I found an example in the docs showing that you really need to use a separate pointer for PySequence_GetItem, and Py_DECREF it after you're done using it. I guess this was just lucky with lists, but numpy arrays are a little too clever for [our] own good.

@jsbtic
Copy link
Author

jsbtic commented Aug 25, 2016

Thanks for the fix!
J.

P.S. I was thinking, could you add the some examples how to convert M x 4 numppy arrays to quaternion arrays and vice versa to the Usage section in the README.md for new users (took some time for me to find out).

E.g.

>>> M = 5
>>> a = np.round(np.random.rand(M, 4),3)
>>> a
array([[ 0.909,  0.671,  0.079,  0.228],
       [ 0.762,  0.492,  0.096,  0.128],
       [ 0.543,  0.574,  0.273,  0.941],
       [ 0.307,  0.434,  0.512,  0.556],
       [ 0.885,  0.538,  0.614,  0.122]])
>>> qs = quaternion.as_quat_array(a)
>>> qs
array([quaternion(0.909, 0.671, 0.079, 0.228),
       quaternion(0.762, 0.492, 0.096, 0.128),
       quaternion(0.543, 0.574, 0.273, 0.941),
       quaternion(0.307, 0.434, 0.512, 0.556),
       quaternion(0.885, 0.538, 0.614, 0.122)], dtype=quaternion)
>>> b = quaternion.as_float_array(qs)
>>> b
array([[ 0.909,  0.671,  0.079,  0.228],
       [ 0.762,  0.492,  0.096,  0.128],
       [ 0.543,  0.574,  0.273,  0.941],
       [ 0.307,  0.434,  0.512,  0.556],
       [ 0.885,  0.538,  0.614,  0.122]])

moble added a commit that referenced this issue Aug 25, 2016
…s conversions

@jsbtic suggested as a comment on issue #25 that the conversions to/from float arrays should be documented on the README.
While adding that and mentioning the other conversions, I realized there was no conversion from quaternions to spherical
coordinates.  I have added that function and a test, along with the documentation.
@moble
Copy link
Owner

moble commented Aug 25, 2016

Good idea. I myself frequently forget how to do these conversions. I've added it to the README. Thanks again!

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