Skip to content

Commit 90ef064

Browse files
committed
Stop embed_rdkit from returning a conformer-less molecule
``EmbedMultipleConfs`` does not only raise on failure - for some strained species it returns normally having embedded zero conformers. ``embed_rdkit`` only guarded the raising path, so in that case it returned an RDMol with no conformers and logged nothing at all. Every consumer of that object then reads an empty list, and the first caller to index it gets ``IndexError: list index out of range`` with no record of where the molecule came from. Reproduced on ``C1#CC1``, ``C1#CCC1`` and ``C1#CC#CC#C1``. Treat zero conformers the same as an exception: log a warning naming the species and return ``None``, which is what the ``RDMol | None`` return annotation already promised and what the exception path already did. All four callers already handle ``None`` and none of them is made worse by receiving it: ``get_force_field_energies`` guards with ``if rd_mol is not None``, ``get_force_field_energies_of_conformers`` returns early on ``None``, ``determine_chirality`` skips on ``rd_mol is None or not rd_mol.GetNumConformers()``, and ``species.get_cheap_conformer`` passes it to ``rdkit_force_field``, which returns empty lists for ``None``. A conformer-less molecule and ``None`` therefore produce the same downstream result, minus the crash and plus a log line.
1 parent 4e51b39 commit 90ef064

2 files changed

Lines changed: 17 additions & 1 deletion

File tree

arc/species/conformers.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1465,7 +1465,8 @@ def embed_rdkit(label, mol, num_confs=None, xyz=None):
14651465
xyz (dict, optional): The 3D coordinates.
14661466
14671467
Returns:
1468-
RDMol | None: An RDKIt molecule with embedded conformers.
1468+
RDMol | None: An RDKIt molecule with embedded conformers,
1469+
or ``None`` if no conformers could be embedded.
14691470
"""
14701471
if num_confs is None and xyz is None:
14711472
raise ConformerError(f'Either num_confs or xyz must be set when calling embed_rdkit() for {label}')
@@ -1482,6 +1483,10 @@ def embed_rdkit(label, mol, num_confs=None, xyz=None):
14821483
except Exception as e:
14831484
logger.warning(f'Could not embed conformers using RDKit for {label}, failed with: {e}')
14841485
return None
1486+
if not rd_mol.GetNumConformers():
1487+
logger.warning(f'Could not embed conformers using RDKit for {label}, '
1488+
f'RDKit returned no conformers without raising an error.')
1489+
return None
14851490
elif xyz is not None:
14861491
rd_conf = Chem.Conformer(rd_mol.GetNumAtoms())
14871492
for i in range(rd_mol.GetNumAtoms()):

arc/species/conformers_test.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,17 @@ def test_embed_rdkit_reports_why_embedding_failed(self):
681681
self.assertIsNone(rd_mol)
682682
self.assertIn('Bad Conformer Id', '\n'.join(captured.output))
683683

684+
def test_embed_rdkit_does_not_return_a_conformer_less_molecule(self):
685+
"""Test that an embedding which yields no conformers returns None rather than an unusable molecule"""
686+
spc = ARCSpecies(label='c-C3H2', smiles='C1#CC1')
687+
with self.assertLogs('arc', level='WARNING') as captured:
688+
rd_mol = conformers.embed_rdkit(label='c-C3H2', mol=spc.mol, num_confs=5)
689+
if rd_mol is not None:
690+
xyzs = conformers.read_rdkit_embedded_conformers(label='c-C3H2', rd_mol=rd_mol)
691+
self.assertIsInstance(xyzs[0], dict)
692+
self.assertIsNone(rd_mol)
693+
self.assertIn('c-C3H2', '\n'.join(captured.output))
694+
684695
def test_rdkit_force_field_abandons_an_optimization_that_raises(self):
685696
"""Test that a raising optimization is logged and attempted once per conformer"""
686697
rd_mol = conformers.embed_rdkit(label='CJ', mol=self.cj_spc.mol, num_confs=1)

0 commit comments

Comments
 (0)