In verbose output, check that the query molecule exists before using it. (#9040)

Co-authored-by: David Cosgrove <david@cozchemix.co.uk>
This commit is contained in:
David Cosgrove
2026-01-08 05:06:52 +00:00
committed by greg landrum
parent 25ce3f3602
commit 96779f2a07
3 changed files with 64 additions and 15 deletions

View File

@@ -10,6 +10,7 @@ rdkit_headers(FMCS.h
rdkit_test(testFMCS testFMCS_Unit.cpp LINK_LIBRARIES
FMCS )
rdkit_catch_test(testFMCSCatch testFMCS_catch_tests.cpp LINK_LIBRARIES FMCS)
if(RDK_BUILD_PYTHON_WRAPPERS)
add_subdirectory(Wrap)

View File

@@ -417,18 +417,21 @@ void MaximumCommonSubgraph::makeInitialSeeds() {
if (!QueryMoleculeSingleMatchedAtom) {
QueryMoleculeSingleMatchedAtom = candQueryMoleculeSingleMatchedAtom;
} else {
QueryMoleculeSingleMatchedAtom = (std::max)(
candQueryMoleculeSingleMatchedAtom,
QueryMoleculeSingleMatchedAtom, [](const Atom *a, const Atom *b) {
if (a->getDegree() != b->getDegree()) {
return (a->getDegree() < b->getDegree());
} else if (a->getFormalCharge() != b->getFormalCharge()) {
return (a->getFormalCharge() < b->getFormalCharge());
} else if (a->getAtomicNum() != b->getAtomicNum()) {
return (a->getAtomicNum() < b->getAtomicNum());
}
return (a->getIdx() < b->getIdx());
});
QueryMoleculeSingleMatchedAtom =
(std::max)(candQueryMoleculeSingleMatchedAtom,
QueryMoleculeSingleMatchedAtom,
[](const Atom *a, const Atom *b) {
if (a->getDegree() != b->getDegree()) {
return (a->getDegree() < b->getDegree());
} else if (a->getFormalCharge() !=
b->getFormalCharge()) {
return (a->getFormalCharge() <
b->getFormalCharge());
} else if (a->getAtomicNum() != b->getAtomicNum()) {
return (a->getAtomicNum() < b->getAtomicNum());
}
return (a->getIdx() < b->getIdx());
});
}
}
}
@@ -1049,7 +1052,10 @@ MCSResult MaximumCommonSubgraph::find(const std::vector<ROMOL_SPTR> &src_mols) {
unsigned int itarget = &tag - &Targets.front();
MatchVectType match;
bool target_matched = SubstructMatch(*tag.Molecule, *res.QueryMol, match);
bool target_matched =
res.QueryMol && SubstructMatch(*tag.Molecule, *res.QueryMol, match)
? true
: false;
if (!target_matched) {
std::cout << "Target " << itarget + 1
<< (target_matched ? " matched " : " MISMATCHED ")
@@ -1246,7 +1252,9 @@ bool MaximumCommonSubgraph::match(Seed &seed) {
for (const auto &tag : Targets) {
unsigned int itarget = &tag - &Targets.front();
#ifdef VERBOSE_STATISTICS_ON
{ ++VerboseStatistics.MatchCall; }
{
++VerboseStatistics.MatchCall;
}
#endif
bool target_matched = false;
if (!seed.MatchResult.empty() && !seed.MatchResult.at(itarget).empty()) {
@@ -1297,7 +1305,9 @@ bool MaximumCommonSubgraph::matchIncrementalFast(Seed &seed,
unsigned int itarget) {
// use and update results of previous match stored in the seed
#ifdef VERBOSE_STATISTICS_ON
{ ++VerboseStatistics.FastMatchCall; }
{
++VerboseStatistics.FastMatchCall;
}
#endif
const auto &target = Targets.at(itarget);
auto &match = seed.MatchResult.at(itarget);

View File

@@ -0,0 +1,38 @@
//
// Copyright (C) David Cosgrove and other RDKit contributors 2026.
//
// @@ All Rights Reserved @@
// This file is part of the RDKit.
// The contents are covered by the terms of the BSD license
// which is included in the file license.txt, found at the root
// of the RDKit source tree.
#include <GraphMol/SmilesParse/SmilesParse.h>
#include "FMCS.h"
#include <catch2/catch_all.hpp>
using namespace RDKit;
TEST_CASE("Github9034") {
std::vector<ROMOL_SPTR> mols;
const char *smi[] = {
"C1CCCCC1",
"C1CCCC1",
};
for (auto &i : smi) {
mols.emplace_back(v2::SmilesParse::MolFromSmiles(i));
}
MCSParameters p;
p.Verbose = true;
p.StoreAll = true;
auto res = findMCS(mols, &p);
// The bug was a crash caused by p.Verbose = true. So the real test
// is whether this completes correctly, but check a few things.
CHECK(res.DegenerateSmartsQueryMolDict.size() == 4);
for (const auto &r : res.DegenerateSmartsQueryMolDict) {
CHECK(r.second->getNumAtoms() == 5);
}
}