-
Notifications
You must be signed in to change notification settings - Fork 118
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
Bring Nanobind. Fix object leak in from_numpy
#2545
Conversation
This comment has been minimized.
This comment has been minimized.
9d8c9e8
to
07d58f6
Compare
✔️ 07d58f6 -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
07d58f6
to
fdc16d0
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cc2a637
to
099255d
Compare
This comment has been minimized.
This comment has been minimized.
099255d
to
bf98304
Compare
✔️ bf98304 -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
from_numpy
from_numpy
This comment has been minimized.
This comment has been minimized.
5c680bb
to
87d6baa
Compare
87d6baa
to
05ed787
Compare
✔️ 05ed787 -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
✔️ f84830b -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
And so we use |
For the git and recursivity I have no idea how to do it better. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one minor comment for clarifications otherwise LGTM!
@ramcdougal has already given a green signal last month.
@nrnhines: you approved this last year. In case you want to take a look again, let us know. Otherwise I think this is ready to go!
thanks a lot @alkino ! 🎉 (& @ferdonline 🍾)
This comment has been minimized.
This comment has been minimized.
✔️ 28bf8ef -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✔️ c218b71 -> Azure artifacts URL |
Quality Gate passedIssues Measures |
✔️ ae200cc -> Azure artifacts URL |
Context
Neuron Python bindings have been implemented by using the Python C API directly. Among the major issues we identified:
Such problems lead to "undead" objects i.e., objects which are no longer in use but due to its wrong ref count they are not garbage collected, keep using valuable memory.
This PR
In this PR we bring nanobind to improve our lives and apply it to solve a case where there was such leaking of undead objects.
TODO
git update --recursive
as this is only needed fornanobind
dllexport
or do it in a separate header file.