Small fix in ring stereo/canonicalization (#8962)

* fix

* update canonicalization watch test

* add ring chirality test

* prune duplicates
This commit is contained in:
Ricardo Rodriguez
2025-12-12 07:56:01 -05:00
committed by GitHub
parent 70540c2eed
commit a8a00a1d99
3 changed files with 44 additions and 43 deletions

View File

@@ -826,40 +826,38 @@ bool updateAtoms(ROMol &mol, const std::vector<unsigned int> &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 &aring = 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 &aring = 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];
}
}
}

View File

@@ -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

View File

@@ -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);