Fixes rdkit#7509 (rdkit#7513)

* add a test * scale pseudo 3d offset * update inchi test --------- Co-authored-by: Greg Landrum <greg.landrum@gmail.com>
rvianello · Jun 14, 2024 · d6171aa · d6171aa
1 parent ea6fbee
commit d6171aa
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 8 deletions.
diff --git a/Code/GraphMol/Chirality.cpp b/Code/GraphMol/Chirality.cpp
@@ -477,8 +477,16 @@ std::optional<Atom::ChiralType> atomChiralTypeFromBondDirPseudo3D(
   auto centerLoc = conf->getAtomPos(atom->getIdx());
   centerLoc.z = 0.0;
   auto refPt = conf->getAtomPos(bondAtom->getIdx());
+
+  // Github #7305: in some odd cases, we get conformers with
+  // weird scalings. In these, we need to scale the 3d offset
+  // or it might be irrelevant or dominate over the coordinates.
+  auto refLength = (centerLoc - refPt).length();
   refPt.z =
       bondDir == Bond::BondDir::BEGINWEDGE ? pseudo3DOffset : -pseudo3DOffset;
+  if (refLength) {
+    refPt.z *= refLength;
+  }
 
   //----------------------------------------------------------
   //
@@ -512,9 +520,14 @@ std::optional<Atom::ChiralType> atomChiralTypeFromBondDirPseudo3D(
       if (nbrBond->getBeginAtomIdx() == atom->getIdx() &&
           (nbrBond->getBondDir() == Bond::BondDir::BEGINWEDGE ||
            nbrBond->getBondDir() == Bond::BondDir::BEGINDASH)) {
+        // scale the 3d offset based on the reference bond here too
         tmpPt.z = nbrBond->getBondDir() == Bond::BondDir::BEGINWEDGE
                       ? pseudo3DOffset
                       : -pseudo3DOffset;
+        if (refLength) {
+          tmpPt.z *= refLength;
+        }
+
       } else {
         tmpPt.z = 0;
       }

diff --git a/Code/GraphMol/catch_chirality.cpp b/Code/GraphMol/catch_chirality.cpp
@@ -5840,4 +5840,40 @@ M  END
     CHECK(mol->getBondWithIdx(13)->getStereo() ==
           Bond::BondStereo::STEREOATROPCCW);
   }
+}
+
+TEST_CASE(
+    "GitHub #7509: atomChiralTypeFromBondDirPseudo3D fails for poorly scaled molecular coordinates") {
+  auto m = R"CTAB(
+     RDKit          2D
+
+  5  4  0  0  0  0  0  0  0  0999 V2000
+   -2.0785    0.0000    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0
+   -0.7794    0.7500    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0
+    0.5196   -0.0000    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0
+    1.8187    0.7500    0.0000 C   0  0  0  0  0  0  0  0  0  0  0  0
+    0.5196   -1.5000    0.0000 O   0  0  0  0  0  0  0  0  0  0  0  0
+  1  2  1  0
+  2  3  1  0
+  3  4  1  1
+  3  5  1  0
+M  END
+)CTAB"_ctab;
+  REQUIRE(m);
+
+  auto at = m->getAtomWithIdx(2);
+  REQUIRE(at->getChiralTag() == Atom::ChiralType::CHI_TETRAHEDRAL_CW);
+
+  auto &pos = m->getConformer().getPositions();
+  std::for_each(pos.begin(), pos.end(),
+                [](RDGeom::Point3D &pos) { pos *= 6.0; });
+
+  // Reset chirality and the original bond direction
+  // (it was stripper after parsing m for the first time)
+  at->setChiralTag(Atom::ChiralType::CHI_UNSPECIFIED);
+  m->getBondBetweenAtoms(2, 3)->setBondDir(Bond::BondDir::BEGINWEDGE);
+
+  MolOps::assignChiralTypesFromBondDirs(*m);
+
+  CHECK(at->getChiralTag() == Atom::ChiralType::CHI_TETRAHEDRAL_CW);
 }
diff --git a/rdkit/Chem/UnitTestInchi.py b/rdkit/Chem/UnitTestInchi.py
@@ -37,14 +37,12 @@
 import unittest
 
 from rdkit import RDConfig, RDLogger
-from rdkit.Chem import (INCHI_AVAILABLE, ForwardSDMolSupplier, MolFromMolBlock,
-                        MolFromSmiles, MolToMolBlock, MolToSmiles, SanitizeMol,
-                        rdDepictor)
+from rdkit.Chem import (INCHI_AVAILABLE, ForwardSDMolSupplier, MolFromMolBlock, MolFromSmiles,
+                        MolToMolBlock, MolToSmiles, SanitizeMol, rdDepictor)
 
 if INCHI_AVAILABLE:
-  from rdkit.Chem import (InchiReadWriteError, InchiToInchiKey,
-                          MolBlockToInchi, MolFromInchi, MolToInchi,
-                          MolToInchiKey)
+  from rdkit.Chem import (InchiReadWriteError, InchiToInchiKey, MolBlockToInchi, MolFromInchi,
+                          MolToInchi, MolToInchiKey)
 
 COLOR_RED = '\033[31m'
 COLOR_GREEN = '\033[32m'
@@ -167,9 +165,9 @@ def test0InchiWritePubChem(self):
 
       fmt = "\n{0}InChI write Summary: {1} identical, {2} suffix variance, {3} reasonable{4}"
       print(fmt.format(COLOR_GREEN, same, diff, reasonable, COLOR_RESET))
-      self.assertEqual(same, 1160)
+      self.assertEqual(same, 1162)
       self.assertEqual(diff, 0)
-      self.assertEqual(reasonable, 21)
+      self.assertEqual(reasonable, 19)
 
   def test1InchiReadPubChem(self):
     for f in self.dataset.values():