Hi everyone,
I starting working on a project that uses pyroot to build a geometry without much prior experience.
The code seems to not trust pyroot at all to not delete objects too early and just uses setownership to false and dumps them in an array that is kept in memory.
The code is older and i understand that memorymanagement differences between c++ and python causes problems at some point.
I want to ask if this is still a problem and if it is still with which objects one doesn’t need to do it.
I assumed that as long as the volumes themself are not deleted any shapes or coordinate transformations shouldn’t be cleaned up by pythons memory management ?
Thanks in advance!
Hello Jerome,
in general, one has to take a decision who will own an object when it’s created from the Python side. A common pattern is to create a PyROOT object on the python side and keeping a reference to it alive, so the object survives also on the C++ side, e.g.:
cppObjects = [ ROOT.Object(x) for x in ... ]
This is especially important if you have objects that refer to each other only by reference or pointer, e.g. in C++:
A a;
B b(&a);
Here, it is important that a survives for b to continue to function. So if in your project the shapes and transformations are created as temporary objects in Python, and TGeo refers to them only by reference or pointer, you have to keep them alive.
When people set ownership to false on the Python side, it’s because they want to transfer ownership to C++. To take this decision however requires a bit of knowledge who is supposed to own which object.
Looking at a few TGeo classes, they seem to mostly take pointers, so intermediate objects should be kept alive when you create them in Python. I would keep them alive by adding them to a big list in Python, but disowning them is another solution …
Hello, thank you for your quick response
Following is used for most objects:
r.SetOwnership(obj, False)
In my understanding this transfers the ownership to the pythonside till all references to it are deleted and is part of a slightly bigger function which tries call RegisterYourself , add the reference to a list and then sets the ownership. This then gets used for basically everything, shapes volumes, transformations. Is this still in the newer versions of pyroot a necessary guard?
Hello,
r.SetOwnership(obj, False) actually removes ownership on the Python side. See also here. After this, nobody owns the object, so it will leak when its Python wrapper is destroyed. Now, if you don’t want the object to leak, somebody needs to take ownership.
I believe that’s why RegisterYourself is used. You cannot have both, Python and TGeoManager owning the object. This sentence from TGeoManager says it all:
TGeoManager is the owner of all geometry objects defined in a session, therefore users must not try to control their deletion.
Therefore, it makes total sense that the ownership on the Python side is removed.
Hello
If i understand this correctly then the function used is somewhat flamed and counterproductive ?
function used:
def register_object(obj, remove_from_root_memory=True):
try:
obj.RegisterYourself()
except:
pass
targets.append(obj)
if(remove_from_root_memory):
r.SetOwnership(obj, False)
this function was part of like 3 years ago init commit so must have been copied from another project probably even older and gets called on everything, shapes,volumes, transformations.
Could something have changed during this time where this function was not udated?
The project only creates a geometry and then exports it to gdml
The function probably does the right thing if remove_from_root_memory is set correctly depending on the case.
But if the geo manager takes ownership I guess you would best do this:
def register_object(obj):
try:
obj.RegisterYourself()
r.SetOwnership(obj, False) # If registration succeeds, Python must give up ownership
except:
pass
targets.append(obj) # Objects that are not registered in the GeoManager will be kept alive using this list
Till now that function was called on everything (except materials) with remove_from_root_memory=True
In my understanding for any transformations one would just want to add them to a list that is kept in memory till done, for volume and shapes one would want to use RegisterYourself() and r.SetOwnership(obj, False) to register it explicitly on the GeoManager and give control to it.
There my question is if for what reason that should be in a try catch pass block as it suppresses errors that indicate that something is not working quite right?
Another question, i assume for materials and mediums it should not be necessary to call r.SetOwnership(obj, False) on if the general structure is
create material → create medium , keep both in a dictionary , use medium for a volume/shape ?
I assume the setup the project im on used has had some memory leak somewhere
I’m afraid I don’t know enough about the TGeo objects to figure out who is the owner. If transformations are not doing RegisterYourself(), you can keep a python list of those around, so they survive until the end of the program. In case they are owned by some C++ object, you need to release them in Python.
For volume and shapes, I agree that you use RegisterYourself and release them from the Python side.
There my question is if for what reason that should be in a try catch pass block as it suppresses errors that indicate that something is not working quite right?
Most likely, that was done because not every object has a RegisterYourself method. If the method call fails, you jump to the catch. That’s why I put the SetOwnership after the RegisterYourself(). If the register succeeds, you need to release in Python. If it fails, execution will go to the catch.
Another question, i assume for materials and mediums it should not be necessary to call
r.SetOwnership(obj, False)on if the general structure is
create material → create medium , keep both in a dictionary , use medium for a volume/shape ?
I don’t know for sure. The question is always whether somebody in C++ will own those, and attempt to delete them. If this is not the case, just keep a Python list or a dictionary of those around.