Skip to content

Fix radius neighbors memory leak. #11056

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

Merged

Conversation

recamshak
Copy link
Contributor

PyArray_UpdateFlags silently ignores the NPY_OWNDATA flag.

Reference Issues/PRs

Fixes #11051

What does this implement/fix? Explain your changes.

Replace the call to PyArray_UpdateFlags that silently ignores the NPY_OWNDATA flag with PyArray_ENABLEFLAGS.

Any other comments?

PyArray_UpdateFlags silently ignores the NPY_OWNDATA flag.
@recamshak recamshak changed the title Fix memory leak. Fix radius neighbors memory leak. May 3, 2018
@jnothman
Copy link
Member

jnothman commented May 3, 2018

@rth, @EemeliSaari please confirm whether this helps, or is a separate issue

@recamshak
Copy link
Contributor Author

recamshak commented May 3, 2018

So far this improves a little bit the issue. Running the script from #11051 (comment) still produces similar output. But calling multiple time radius_neighbors does not increase memory usage anymore.

@rth
Copy link
Member

rth commented May 3, 2018

Thanks @recamshak !

Running the script from #11051 (comment) still produces similar output. But calling multiple time radius_neighbors does not increase memory usage anymore.

For me, this doesn't seem to change the output of the benchmark script in both cases, but I still think it's an improvement over the current version.

Side note: I find https://stackoverflow.com/a/23873586/1791279 helpful to understand the general mechanism on a small example. Though when I tried to increase N = 500000000 in test() which results in a 400MB array, and measure the memory usage with memory_profiler and htop I don't even see the allocation which is very strange..

@jnothman jnothman merged commit 9a9cf19 into scikit-learn:master May 3, 2018
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

Successfully merging this pull request may close these issues.

NearestNeighbors radius_neighbors memory leaking
3 participants