Skip to content

Commit

Permalink
make sure outdated ring fusion information is recomputed before attem…
Browse files Browse the repository at this point in the history 
…pting to use it (rdkit#7274)

Co-authored-by: ptosco <paolo.tosco@novartis.com>
  • Loading branch information
@ptosco
ptosco and ptosco authored Mar 20, 2024
1 parent 4f1b88a commit f543172
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 13 deletions.
29 changes: 16 additions & 13 deletions Code/GraphMol/RingInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,25 +195,22 @@ unsigned int RingInfo::addRing(const INT_VECT &atomIndices,
}

bool RingInfo::isRingFused(unsigned int ringIdx) {
PRECONDITION(ringIdx < d_bondRings.size(), "ringIdx out of bounds");
if (d_fusedRings.empty()) {
initFusedRings();
}
return d_fusedRings.at(ringIdx).any();
initFusedRings();
PRECONDITION(ringIdx < d_fusedRings.size(), "ringIdx out of bounds");
return d_fusedRings[ringIdx].any();
}

bool RingInfo::areRingsFused(unsigned int ring1Idx, unsigned int ring2Idx) {
PRECONDITION(ring1Idx < d_bondRings.size(), "ring1Idx out of bounds");
PRECONDITION(ring2Idx < d_bondRings.size(), "ring2Idx out of bounds");
if (d_fusedRings.empty()) {
initFusedRings();
}
return d_fusedRings.at(ring1Idx).test(ring2Idx);
initFusedRings();
PRECONDITION(ring1Idx < d_fusedRings.size(), "ring1Idx out of bounds");
PRECONDITION(ring2Idx < d_fusedRings.size(), "ring2Idx out of bounds");
return d_fusedRings[ring1Idx].test(ring2Idx);
}

unsigned int RingInfo::numFusedBonds(unsigned int ringIdx) {
PRECONDITION(ringIdx < d_bondRings.size(), "ringIdx out of bounds");
if (d_numFusedBonds.empty()) {
if (d_numFusedBonds.size() != d_bondRings.size()) {
d_numFusedBonds.clear();
d_numFusedBonds.resize(d_bondRings.size(), 0);
for (unsigned int ri = 0; ri < d_bondRings.size(); ++ri) {
d_numFusedBonds[ri] += std::count_if(
Expand All @@ -225,12 +222,14 @@ unsigned int RingInfo::numFusedBonds(unsigned int ringIdx) {
}

unsigned int RingInfo::numFusedRingNeighbors(unsigned int ringIdx) {
initFusedRings();
PRECONDITION(ringIdx < d_fusedRings.size(), "ringIdx out of bounds");
return d_fusedRings[ringIdx].count();
}

std::vector<unsigned int> RingInfo::fusedRingNeighbors(unsigned int ringIdx) {
PRECONDITION(ringIdx < d_bondRings.size(), "ringIdx out of bounds");
initFusedRings();
PRECONDITION(ringIdx < d_fusedRings.size(), "ringIdx out of bounds");
std::vector<unsigned int> res;
res.reserve(d_fusedRings[ringIdx].count());
for (unsigned int i = 0; i < d_fusedRings[ringIdx].size(); ++i) {
Expand All @@ -242,6 +241,10 @@ std::vector<unsigned int> RingInfo::fusedRingNeighbors(unsigned int ringIdx) {
}

void RingInfo::initFusedRings() {
if (d_fusedRings.size() == d_bondRings.size()) {
return;
}
d_fusedRings.clear();
if (d_bondRings.empty()) {
return;
}
Expand Down
39 changes: 39 additions & 0 deletions Code/GraphMol/Wrap/rough_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8029,6 +8029,45 @@ def testAddStereoAnnotations(self):
self.assertEqual(mol.GetAtomWithIdx(5).GetProp("atomNote"), "abs (S)")
self.assertEqual(mol.GetAtomWithIdx(3).GetProp("atomNote"), "and2")

def testIsRingFused(self):
molOrig = Chem.MolFromSmiles("C1C(C2CC3CCCCC3C12)C1CCCCC1")
mol = Chem.RWMol(molOrig)
ri = mol.GetRingInfo()
self.assertEqual(ri.NumRings(), 4)
fusedRings = [ri.IsRingFused(i) for i in range(ri.NumRings())]
self.assertEqual(fusedRings.count(True), 3)
self.assertEqual(fusedRings.count(False), 1)
atoms = mol.GetSubstructMatch(Chem.MolFromSmarts("[$(C1CCC1)]-@[$(C1CCCCC1)]"))
mol.RemoveBond(*atoms)
Chem.SanitizeMol(mol)
self.assertEqual(Chem.MolToSmiles(mol), "C1CCC(CC2CCC2C2CCCCC2)CC1")
self.assertEqual(ri.NumRings(), 3)
fusedRings = [ri.IsRingFused(i) for i in range(ri.NumRings())]
self.assertEqual(fusedRings.count(True), 0)
self.assertEqual(fusedRings.count(False), 3)
mol = Chem.RWMol(molOrig)
ri = mol.GetRingInfo()
self.assertEqual(ri.NumRings(), 4)
fusedRings = [ri.IsRingFused(i) for i in range(ri.NumRings())]
self.assertEqual(fusedRings.count(True), 3)
self.assertEqual(fusedRings.count(False), 1)
fusedBonds = [ri.NumFusedBonds(i) for i in range(ri.NumRings())]
self.assertEqual(fusedBonds.count(0), 1)
self.assertEqual(fusedBonds.count(1), 2)
self.assertEqual(fusedBonds.count(2), 1)
atoms = mol.GetSubstructMatch(Chem.MolFromSmarts("[$(C1CCCCC1-!@[CX4;R1;r4])].[$(C1C(-!@[CX4;R1;r6])CC1)]"))
mol.AddBond(*atoms, Chem.BondType.SINGLE)
Chem.SanitizeMol(mol)
self.assertEqual(Chem.MolToSmiles(mol), "C1CCC2C(C1)CC1C2C2C3CCCCC3C12")
self.assertEqual(ri.NumRings(), 5)
fusedRings = [ri.IsRingFused(i) for i in range(ri.NumRings())]
self.assertEqual(fusedRings.count(True), 5)
self.assertEqual(fusedRings.count(False), 0)
fusedBonds = [ri.NumFusedBonds(i) for i in range(ri.NumRings())]
self.assertEqual(fusedBonds.count(0), 0)
self.assertEqual(fusedBonds.count(1), 2)
self.assertEqual(fusedBonds.count(2), 3)


if __name__ == '__main__':
if "RDTESTCASE" in os.environ:
Expand Down
73 changes: 73 additions & 0 deletions Code/GraphMol/molopstest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8475,6 +8475,78 @@ void testHasQueryHs() {

BOOST_LOG(rdInfoLog) << "\tdone" << std::endl;
}

void testIsRingFused() {
BOOST_LOG(rdWarningLog) << "-----------------------\n Testing isRingFused "
<< std::endl;
auto molOrig = "C1C(C2CC3CCCCC3C12)C1CCCCC1"_smiles;
{
RWMol mol(*molOrig);
auto ri = mol.getRingInfo();
TEST_ASSERT(ri->numRings() == 4);
boost::dynamic_bitset<> fusedRings(ri->numRings());
for (size_t i = 0; i < ri->numRings(); ++i) {
fusedRings.set(i, ri->isRingFused(i));
}
TEST_ASSERT(fusedRings.count() == 3);
TEST_ASSERT(fusedRings.size() - fusedRings.count() == 1);
auto query = "[$(C1CCC1)]-@[$(C1CCCCC1)]"_smarts;
MatchVectType matchVect;
SubstructMatch(mol, *query, matchVect);
TEST_ASSERT(matchVect.size() == 2);
mol.removeBond(matchVect.at(0).second, matchVect.at(1).second);
MolOps::sanitizeMol(mol);
TEST_ASSERT(MolToSmiles(mol) == "C1CCC(CC2CCC2C2CCCCC2)CC1");
TEST_ASSERT(ri->numRings() == 3);
fusedRings.resize(ri->numRings());
for (size_t i = 0; i < ri->numRings(); ++i) {
fusedRings.set(i, ri->isRingFused(i));
}
TEST_ASSERT(fusedRings.count() == 0);
TEST_ASSERT(fusedRings.size() - fusedRings.count() == 3);
}
{
RWMol mol(*molOrig);
auto ri = mol.getRingInfo();
TEST_ASSERT(ri->numRings() == 4);
boost::dynamic_bitset<> fusedRings(ri->numRings());
for (size_t i = 0; i < ri->numRings(); ++i) {
fusedRings.set(i, ri->isRingFused(i));
}
TEST_ASSERT(fusedRings.count() == 3);
TEST_ASSERT(fusedRings.size() - fusedRings.count() == 1);
std::vector<unsigned int> fusedBonds(ri->numRings());
for (size_t i = 0; i < ri->numRings(); ++i) {
fusedBonds[i] = ri->numFusedBonds(i);
}
TEST_ASSERT(std::count(fusedBonds.begin(), fusedBonds.end(), 0) == 1);
TEST_ASSERT(std::count(fusedBonds.begin(), fusedBonds.end(), 1) == 2);
TEST_ASSERT(std::count(fusedBonds.begin(), fusedBonds.end(), 2) == 1);
auto query =
"[$(C1CCCCC1-!@[CX4;R1;r4])].[$(C1C(-!@[CX4;R1;r6])CC1)]"_smarts;
MatchVectType matchVect;
SubstructMatch(mol, *query, matchVect);
TEST_ASSERT(matchVect.size() == 2);
mol.addBond(matchVect.at(0).second, matchVect.at(1).second, Bond::SINGLE);
MolOps::sanitizeMol(mol);
TEST_ASSERT(MolToSmiles(mol) == "C1CCC2C(C1)CC1C2C2C3CCCCC3C12");
TEST_ASSERT(ri->numRings() == 5);
fusedRings.resize(ri->numRings());
for (size_t i = 0; i < ri->numRings(); ++i) {
fusedRings.set(i, ri->isRingFused(i));
}
TEST_ASSERT(fusedRings.count() == 5);
TEST_ASSERT(fusedRings.size() - fusedRings.count() == 0);
fusedBonds.resize(ri->numRings());
for (size_t i = 0; i < ri->numRings(); ++i) {
fusedBonds[i] = ri->numFusedBonds(i);
}
TEST_ASSERT(std::count(fusedBonds.begin(), fusedBonds.end(), 0) == 0);
TEST_ASSERT(std::count(fusedBonds.begin(), fusedBonds.end(), 1) == 2);
TEST_ASSERT(std::count(fusedBonds.begin(), fusedBonds.end(), 2) == 3);
}
}

int main() {
RDLog::InitLogs();
// boost::logging::enable_logs("rdApp.debug");
Expand Down Expand Up @@ -8592,5 +8664,6 @@ int main() {
testGet3DDistanceMatrix();
testGithub5099();
testHasQueryHs();
testIsRingFused();
return 0;
}

0 comments on commit f543172

Please sign in to comment.