Skip to content
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

RDKit test failure #4813

Closed
RMeli opened this issue Dec 1, 2024 · 3 comments · Fixed by #4824
Closed

RDKit test failure #4813

RMeli opened this issue Dec 1, 2024 · 3 comments · Fixed by #4824
Labels
testing upstream issue in a dependency

Comments

@RMeli
Copy link
Member

RMeli commented Dec 1, 2024

RDKit test failures started to show up in CI:

>               Chem.AssignStereochemistryFrom3D(mol)
E               RuntimeError: Cannot normalize a zero length vector

A new version of RDKit landed a couple of days ago: https://github.com/rdkit/rdkit/releases.

@orbeckst orbeckst added testing upstream issue in a dependency labels Dec 4, 2024
@RMeli
Copy link
Member Author

RMeli commented Dec 4, 2024

I can reproduce it locally (macOS 15) with rdkit==2024.9.3 but it works with rdkit==2024.9.2.

@RMeli
Copy link
Member Author

RMeli commented Dec 4, 2024

Reproducer:

import rdkit.Chem as Chem
import MDAnalysis as mda
from io import StringIO

mol = Chem.MolFromSmiles("O=S(C)(C)=NC")
mol = Chem.AddHs(mol)
pdb = Chem.MolToPDBBlock(mol)
print(pdb)
u = mda.Universe(StringIO(pdb), format="PDB")
ag = u.select_atoms("all")
conf = Chem.Conformer(mol.GetNumAtoms())
for atom in mol.GetAtoms():
    idx = atom.GetIdx()
    xyz = ag.positions[idx].astype(float)
    print(xyz)
    conf.SetAtomPosition(idx, xyz)
mol.AddConformer(conf)
Chem.AssignStereochemistryFrom3D(mol)

The positions are all zero in the PDB file, and therefore the same is true for the coordinates of the conformer. Something must have changed within AssignStereochemistryFrom3D where a normalization (on distances?) has been added.

@RMeli
Copy link
Member Author

RMeli commented Dec 5, 2024

@cbouy do you have any idea of what changed? Something definitely changed in RDKit, but I feel we are hitting an issue because we were relying on a bug/implementation detail of RDKit: the function AssignStereochemistryFrom3D is expected to user 3D information to assign the stereochemistry, while all coordinates of the conformer are [0., 0., 0.].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing upstream issue in a dependency
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants