Memory leak with Chem.Atom() constructor

[AlanKerstjens]

The following bit of code quickly gobbles up all available memory. Maybe I'm missing something but it looks like a memory leak.

from rdkit import Chem

while True:
    atom = Chem.Atom(0)

RDKit version: 2019.03.3.
Platform: Ubuntu (release 18.04)

I'm trying to use this code to create pseudoatoms. Is there perhaps any alternative?

[bp-kelley]

This is an unfortunate by product of wrapping a c++ library. An Atom is designed to be cleaned up when a molecule is destructed. The overhead of bookkeeping if an atom is "owned" by a molecule hasn't been implemented in Python since the basic design of RDKit implies that an Atom really should be managed by a molecule. To do this in python, you'll need to add the atom to a molecule.

>>> from rdkit.Chem import RWMol, Atom
>>> while True:
...   m = RWMol()
...   m.AddAtom(Atom(6))

We probably should have a function:

  atom = m.AddAtom(6)

for quality of life.

[AlanKerstjens]

Thank you for your swift reply!

The usage example you provided is very similar to what I would like to do: create an Atom and add it to a RWMol. But even when the RWMol is destructed the memory associated with an explicitly constructed Atom doesn't seem to be recovered. For instance, when running the code you provided I run into the same memory issue.

By contrast, when adding an Atom from a different source (e.g. another Mol) this doesn't happen:

from rdkit import Chem

benzene = Chem.MolFromSmiles("c1ccccc1")

while True:
    mol = Chem.RWMol()
    mol.AddAtom(benzene.GetAtomWithIdx(0))

I also tried wrapping the molecule creation in a function and manually deleting the reference to the molecule just to make sure that the refcount drops to 0 and that the Mol is destructed, but I can't seem to free up the memory.

[bp-kelley]

Atoms are definitely leaking. I'm just unsure of why. The reason that adding an atom from another molecule is working, is that the other molecule has ownership and deletes the atom.

This is a silly stop gap for now, I'll file it as a bug and investigate it.

[AlanKerstjens]

Thanks for the follow up.

In the meantime, if anyone else runs into this problem, it seems to be possible to initialize only one ownerless Atom and then re-use this Atom whenever needed. The downside is that if one needs to use different types of Atoms for each molecule it requires some extra function calls. It isn't very elegant but it gets the job done.

from rdkit import Chem

atom = Chem.Atom(0)

while True:
    mol = Chem.RWMol()
    atom_idx = mol.AddAtom(atom)
    mol.GetAtomWithIdx(atom_idx).SetAtomicNum(6)

[bp-kelley]

@AlanKerstjens

Yeah, that's suboptimal :) Although you could do:

C = Atom(6)
N = Atom(7)

mol.AddAtom(C)

which might be more literal.

I think I'll pass this by Greg:

mol.AddAtom(6)

to avoid creating a leaky atom at all.

[greglandrum]

I don't think Chem.Atom(6) should be something that leaks. The change to avoid it (#2639) is straightforward.