diff --git a/Code/GraphMol/FindStereo.cpp b/Code/GraphMol/FindStereo.cpp index 83304732f..72dbfda3c 100644 --- a/Code/GraphMol/FindStereo.cpp +++ b/Code/GraphMol/FindStereo.cpp @@ -826,40 +826,38 @@ bool updateAtoms(ROMol &mol, const std::vector &aranks, // if this was creating possible ring stereo, update that info now if (possibleRingStereoAtoms[aidx]) { - --possibleRingStereoAtoms[aidx]; - if (!possibleRingStereoAtoms[aidx]) { - needAnotherRound = true; - // we're no longer in any ring with possible ring stereo. Go - // update all the other atoms/bonds in rings that we're in: - for (unsigned int ridx = 0; - ridx < mol.getRingInfo()->atomRings().size(); ++ridx) { - const auto å = mol.getRingInfo()->atomRings()[ridx]; - unsigned int nHere = 0; - for (auto raidx : aring) { - // Ring stereo changed, so un-fix atoms in this ring so we can - // recheck them in the next iteration, for the case that they - // are no longer after the current atom was declared - // non-chiral - fixedAtoms[raidx] = false; - nHere += (possibleRingStereoAtoms[raidx] > 0); - } - if (nHere <= 1) { - // if the ring can't transmit stereo anymore, update the - // counts - if (nHere == 1) { - // update the last potential ring stereo atom in the ring, - // since it can't have ring stereo alone. - for (auto raidx : aring) { - if (possibleRingStereoAtoms[raidx]) { - --possibleRingStereoAtoms[raidx]; - break; - } + possibleRingStereoAtoms[aidx] = 0; + needAnotherRound = true; + // we're no longer in any ring with possible ring stereo. Go + // update all the other atoms/bonds in rings that we're in: + for (unsigned int ridx = 0; + ridx < mol.getRingInfo()->atomRings().size(); ++ridx) { + const auto å = mol.getRingInfo()->atomRings()[ridx]; + unsigned int nHere = 0; + for (auto raidx : aring) { + // Ring stereo changed, so un-fix atoms in this ring so we can + // recheck them in the next iteration, for the case that they + // are no longer after the current atom was declared + // non-chiral + fixedAtoms[raidx] = false; + nHere += (possibleRingStereoAtoms[raidx] > 0); + } + if (nHere <= 1) { + // if the ring can't transmit stereo anymore, update the + // counts + if (nHere == 1) { + // update the last potential ring stereo atom in the ring, + // since it can't have ring stereo alone. + for (auto raidx : aring) { + if (possibleRingStereoAtoms[raidx]) { + --possibleRingStereoAtoms[raidx]; + break; } } - for (auto rbidx : mol.getRingInfo()->bondRings()[ridx]) { - if (possibleRingStereoBonds[rbidx]) { - --possibleRingStereoBonds[rbidx]; - } + } + for (auto rbidx : mol.getRingInfo()->bondRings()[ridx]) { + if (possibleRingStereoBonds[rbidx]) { + --possibleRingStereoBonds[rbidx]; } } } diff --git a/Code/GraphMol/catch_canon.cpp b/Code/GraphMol/catch_canon.cpp index 84c692092..afbdd29cb 100644 --- a/Code/GraphMol/catch_canon.cpp +++ b/Code/GraphMol/catch_canon.cpp @@ -1172,8 +1172,7 @@ TEST_CASE("Canonicalization issues watch (see GitHub Issue #8775)") { {R"smi(C1=C\CCCCCC/C=C/C=C/1)smi", true, true}, // #8759 {R"smi(O=C=NC1=CC2C3=C(C=C1)C2=C(N=C=O)C=C3)smi", false, false}, // #8721 {R"smi(O=C(c1ccccc1C(=O)N1C(=O)c2ccccc2C1=O)N1C(=O)c2ccccc2C1=O)smi", - false, false}, // #8721 - {R"smi(O=C=NC1=CC2C3=C(C=C1)C2=C(N=C=O)C=C3)smi", false, false}, // #8721 + false, false}, // #8721 {R"smi(O=[N+]([O-])c1cc/c2c(c1)=C(c1ccccc1)/N=c1\\ccc([N+](=O)[O-])cc1=C(c1ccccc1)/N=2)smi", false, true}, // #8721 {R"smi(C=Cc1c(C)/c2[n-]c1=C=c1[n-]/c(c(CC)c1C)=C\\c1[n-]c3c(c1C)C(=O)[C@H](C(=O)OC)/C3=C1/[NH+]=C(/C=2)[C@@H](C)[C@@H]1CCC(=O)OC/C=C(\\C)CCC[C@H](C)CCC[C@H](C)CCCC(C)C.[Mg+2])smi", @@ -1220,11 +1219,6 @@ TEST_CASE("Canonicalization issues watch (see GitHub Issue #8775)") { {R"smi([H]/N=C(C=C)\C(/N=C\[O-])=N\[H])smi", false, true}, // #7759 {R"smi([H]N=C(/N=C\[O-])/C(C=C)=N\[H])smi", true, true}, // #7759 {R"smi([H]/N=C(/C=C)C(=N)/N=C\[O-])smi", true, true}, // #7759 - {R"smi([H]/N=C(/C=C)C(=N)/N=C\[O-])smi", true, true}, // #7759 - {R"smi([C@H]12[C@H]3[C@@H]4[C@H]5[C@@H]([C@H]1N24)N53)smi", false, - true}, // #7759 - {R"smi([C@H]12[C@H]3[C@H]4[C@@H]5[C@@H]([C@H]1N25)N34)smi", false, - true}, // #7759 {R"smi([C@H]12[C@H]3[C@@H]4[C@H]5[C@@H]([C@H]1N24)N53)smi", false, true}, // #7759 {R"smi([C@H]12[C@H]3[C@H]4[C@@H]5[C@@H]([C@H]1N25)N34)smi", false, @@ -1234,12 +1228,10 @@ TEST_CASE("Canonicalization issues watch (see GitHub Issue #8775)") { {R"smi([CH]1O[C@H]2CN(C2)[C@]12CC2)smi", false, true}, // #7759 {R"smi([CH]1[C@H]2C[C@H](C2)[C@]12CCC2)smi", false, true}, // #7759 {R"smi([CH]1[C@H]2C[C@H](C2)[C@@]12COC2)smi", false, true}, // #7759 - {R"smi(O=C1[C]2C[C@H](C2)[C@@]12CC2)smi", false, false}, // #7759 + {R"smi(O=C1[C]2C[C@H](C2)[C@@]12CC2)smi", false, true}, // #7759 {R"smi(O=C1[C@H]2C[C](C2)[C@@]12CC2)smi", false, true}, // #7759 {R"smi(C1[C@H]2[C@@H]3C[C@H]4[C@@H]3[C@@H]1[C@@H]24)smi", false, true}, // #7759 - {R"smi([C@H]12[C@H]3[C@@H]4[C@H]5[C@@H]([C@H]1N24)N53)smi", false, - true}, // #7759 {R"smi(C[C@]12[C@@H]3[C@@H]1C[C@H]1[C@H]2[C@]31C)smi", false, true}, // #7759 {R"smi([O][C@]12C[C@H](C1)[C@@]1(CC1)C2)smi", false, true}, // #7759 @@ -1301,7 +1293,7 @@ TEST_CASE("Canonicalization issues watch (see GitHub Issue #8775)") { {R"smi([CH-]1O[C@H]2C[C@H](C2)[C@]12CC2)smi", false, true}, // #7759 {R"smi([CH-]1[C@H]2C[C@H](C2)[C@]12CCC2)smi", false, true}, // #7759 {R"smi([CH-]1[C@H]2C[C@H](C2)[C@@]12COC2)smi", false, true}, // #7759 - {R"smi([O-]C1=C2C[C@H](C2)[C@@]12CC2)smi", false, false}, // #7759 + {R"smi([O-]C1=C2C[C@H](C2)[C@@]12CC2)smi", false, true}, // #7759 {R"smi(O=C1[C@H]2C[C-](C2)[C@@]12CC2)smi", false, true}, // #7759 {R"smi([O-][C@]12C[C@H](C1)[C@@]1(CC1)C2)smi", false, true}, // #7759 {R"smi([CH2-][C@]12C[C@H](C1)[C@]1(CC1)C2)smi", false, true}, // #7759 diff --git a/Code/GraphMol/catch_chirality.cpp b/Code/GraphMol/catch_chirality.cpp index 05439052b..6f298a0e7 100644 --- a/Code/GraphMol/catch_chirality.cpp +++ b/Code/GraphMol/catch_chirality.cpp @@ -1251,6 +1251,17 @@ TEST_CASE("ring stereo finding is overly aggressive", "[chirality][bug]") { Chirality::findPotentialStereo(*mol, cleanIt, flagPossible); CHECK(stereoInfo.size() == 2); } + SECTION("bad ring chirality due to pair being in 2 rings") { + // from canonicalization watch test. + + auto mol = "[O-]C1=C2C[C@H](C2)[C@@]12CC2"_smiles; + REQUIRE(mol); + bool cleanIt = true; + bool flagPossible = true; + auto stereoInfo = + Chirality::findPotentialStereo(*mol, cleanIt, flagPossible); + CHECK(stereoInfo.size() == 0); + } SECTION( "Removal of stereoatoms requires removing CIS/TRANS when using legacy stereo") { UseLegacyStereoPerceptionFixture reset_stereo_perception(false);