completed

rdkit · May 1, 2024 · d91ec9d · d91ec9d
1 parent 225f0d1
commit d91ec9d
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 22 deletions.
diff --git a/Code/GraphMol/Atom.cpp b/Code/GraphMol/Atom.cpp
@@ -312,33 +312,40 @@ unsigned int Atom::getTotalValence() const {
 
 namespace {
 
-unsigned int calcAtomChgAndElecEnv(const Atom &atom, int &chg, int &effectiveChg, int &nOuterElecs, int &nElectronRichNbrs, bool addDativeBondContrib) {
+unsigned int calcAtomChg(const Atom &atom, int &chg, int &effectiveChg, int &effectiveValenceChgContrib, bool addDativeBondContrib) {
   auto atomicNum = atom.getAtomicNum();
   int dativeNbrChg = 0;
-  nElectronRichNbrs = 0;
-  nOuterElecs = PeriodicTable::getTable()->getNouterElecs(atomicNum);
+  int nElectronRichNbrs = 0;
+  int nOuterElecs = PeriodicTable::getTable()->getNouterElecs(atomicNum);
   for (const auto bnd : atom.getOwningMol().atomBonds(&atom)) {
     auto nbrAtom = bnd->getOtherAtom(&atom);
     // dative bonds from nbr to atom contribute neighbor formal charge to atom
-    if (addDativeBondContrib && (bnd->getBondType() == Bond::DATIVE ||
-         bnd->getBondType() == Bond::DATIVEONE) &&
+    bool isBondDative = (bnd->getBondType() == Bond::DATIVE ||
+         bnd->getBondType() == Bond::DATIVEONE);
+    if (addDativeBondContrib && isBondDative &&
         bnd->getEndAtom() == &atom) {
       dativeNbrChg += nbrAtom->getFormalCharge();
     }
     auto nbrAtomicNum = nbrAtom->getAtomicNum();
-    if (PeriodicTable::getTable()->getNouterElecs(nbrAtomicNum) > nOuterElecs) {
+    if (!(isBondDative && bnd->getBeginAtom() == &atom) && PeriodicTable::getTable()->getNouterElecs(nbrAtomicNum) > nOuterElecs) {
       ++nElectronRichNbrs;
     }
   }
   chg = atom.getFormalCharge() + dativeNbrChg;
   effectiveChg = chg;
-  if (isEarlyAtom(atomicNum)) {
+  auto atomIsEarly = isEarlyAtom(atomicNum);
+  if (atomIsEarly) {
     effectiveChg = -chg;  // <- the usual correction for early atoms
   }
   // special case for carbon - see GitHub #539
   if (atomicNum == 6 && effectiveChg > 0) {
     effectiveChg = -chg;
   }
+  effectiveValenceChgContrib = chg;
+  if (nOuterElecs >= 4 && !atomIsEarly &&
+      (atomicNum <= 10 || chg > 0 || nElectronRichNbrs < -chg)) {
+    effectiveValenceChgContrib = -chg;
+  }
   return atomicNum;
 }
 
@@ -348,9 +355,8 @@ int calculateExplicitValence(const Atom &atom, bool strict, bool checkIt) {
   double accum = 0.0;
   int chg;
   int effectiveChg;
-  int nOuterElecs;
-  int nElectronRichNbrs;
-  auto atomicNum = calcAtomChgAndElecEnv(atom, chg, effectiveChg, nOuterElecs, nElectronRichNbrs, true);
+  int effectiveValenceChgContrib;
+  auto atomicNum = calcAtomChg(atom, chg, effectiveChg, effectiveValenceChgContrib, true);
   for (const auto bnd : atom.getOwningMol().atomBonds(&atom)) {
     accum += bnd->getValenceContrib(&atom);
   }
@@ -426,11 +432,6 @@ int calculateExplicitValence(const Atom &atom, bool strict, bool checkIt) {
     // [AlF6]3-, and therefore gain negative formal charge
     // (e.g., hexafluorophosphate [PF6]-, hexafluorosilicate [SiF6]2-,
     // hexafluoroantimonate [AsF6]-, bismuthate [BiO3]-)
-    int effectiveValenceChgContrib = chg;
-    if (nOuterElecs >= 4 &&
-        (atomicNum <= 10 || chg > 0 || nElectronRichNbrs < -chg)) {
-      effectiveValenceChgContrib = -chg;
-    }
     int effectiveValence = res + effectiveValenceChgContrib;
     const auto &valens = PeriodicTable::getTable()->getValenceList(atomicNum);
 
@@ -469,9 +470,8 @@ int calculateImplicitValence(const Atom &atom, bool strict, bool checkIt) {
   // special cases
   int chg;
   int effectiveChg;
-  int nOuterElecs;
-  int nElectronRichNbrs;
-  auto atomicNum = calcAtomChgAndElecEnv(atom, chg, effectiveChg, nOuterElecs, nElectronRichNbrs, false);
+  int effectiveValenceChgContrib;
+  auto atomicNum = calcAtomChg(atom, chg, effectiveChg, effectiveValenceChgContrib, false);
   if (atomicNum == 0) {
     return 0;
   }
@@ -600,10 +600,8 @@ int calculateImplicitValence(const Atom &atom, bool strict, bool checkIt) {
     // and be able to add hydrogens
     res = -1;
     for (auto vi = valens.begin(); vi != valens.end() && *vi >= 0; ++vi) {
-      int tot = *vi + effectiveChg;
-      std::cerr << "1) *vi " << *vi << ", explicitPlusRadV " << explicitPlusRadV << ", effectiveChg " << effectiveChg << ", tot " << tot << std::endl;
+      int tot = *vi - effectiveValenceChgContrib;
       if (explicitPlusRadV <= tot) {
-        std::cerr << "2) *vi " << *vi << ", explicitPlusRadV " << explicitPlusRadV << ", effectiveChg " << effectiveChg << ", tot " << tot << std::endl;
         res = tot - explicitPlusRadV;
         break;
       }

diff --git a/rdkit/Chem/UnitTestSmiles.py b/rdkit/Chem/UnitTestSmiles.py
@@ -98,7 +98,7 @@ def testRings2(self):
   def testProblems(self):
     " testing molecules which have been problematic "
     smiList = [
-      ('[Al+3]CCC', ('CCC[Al+3]', 'C(C)(C[Al+3])')),
+      ('[Al+2]CCC', ('CCC[Al+2]', 'C(C)(C[Al+2])')),
       ('C(=O)(Cl)CC(=O)Cl', ('ClC(CC(Cl)=O)=O', 'C(Cl)(=O)CC(=O)Cl', 'C(Cl)(=O)CC(Cl)=O')),
       ('C(=O)(Cl)c1ccc(C(=O)Cl)cc1', ('O=C(Cl)c1ccc(cc1)C(Cl)=O', 'C(Cl)(=O)C1=CC=C(C=C1)C(Cl)=O',
                                       'ClC(=O)c1ccc(cc1)C(=O)Cl')),