Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions nvmolkit/tests/test_substructure.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,23 @@ def test_buffer_overflow_rdkit_fallback(self):
assert len(results[0][0]) == 10, f"Should get all 10 matches via RDKit fallback, got {len(results[0][0])}"
assert matches_equal(results[0][0], rdkit_matches), "Matches should be identical to RDKit results"

@pytest.mark.parametrize("preprocessing_threads", [1, 2, -1])
def test_disconnected_smarts_raises(self, preprocessing_threads: int):
"""Disconnected (fragment) SMARTS must raise rather than crash the process.

Regression test for GH issue 203: with more than one preprocessing
thread the validation error escaped the OpenMP region and terminated the
process. The unsupported-query error must surface as a RuntimeError for
every preprocessing-thread count.
"""
targets = [Chem.MolFromSmiles(smi) for smi in ("CC", "CCC", "CCCC")]
queries = [Chem.MolFromSmarts("C.C")]

config = SubstructSearchConfig(preprocessingThreads=preprocessing_threads)

with pytest.raises(RuntimeError, match="disconnected"):
hasSubstructMatch(targets, queries, config)

def test_no_match_possible(self):
"""Test when no match is possible."""
targets = [Chem.MolFromSmiles("CCCC")]
Expand Down
2 changes: 1 addition & 1 deletion src/substruct/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ add_library(molecules molecules.cpp)
target_link_libraries(
molecules
PUBLIC device_vector flatBitVect OpenMP::OpenMP_CXX packed_bonds device
PRIVATE ${RDKit_LIBS} nvtx)
PRIVATE ${RDKit_LIBS} nvtx openmp_helpers)

add_library(substruct_kernels substruct_kernels.cu)
target_link_libraries(
Expand Down
17 changes: 15 additions & 2 deletions src/substruct/molecules.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include <algorithm>
#include <cstring>
#include <exception>
#include <functional>
#include <stdexcept>
#include <string>
Expand All @@ -33,6 +34,7 @@
#include "src/substruct/substruct_debug.h"
#include "src/substruct/substruct_types.h"
#include "src/utils/nvtx.h"
#include "src/utils/openmp_helpers.h"
#include "src/utils/rdkit_compat.h"

namespace nvMolKit {
Expand Down Expand Up @@ -2128,6 +2130,12 @@ MoleculesHost buildQueryBatchParallel(const std::vector<const RDKit::ROMol*>& mo
threadBatches[t].reserve(molsPerThread, atomsPerThread);
}

// addQueryToBatch validates each query and throws on unsupported inputs (e.g.
// disconnected SMARTS). An exception escaping the OpenMP region would call
// std::terminate, so capture the first failure and rethrow after the region
// joins.
detail::OpenMPExceptionRegistry exceptionRegistry;

#pragma omp parallel num_threads(numThreads)
{
const int tid = omp_get_thread_num();
Expand All @@ -2136,10 +2144,15 @@ MoleculesHost buildQueryBatchParallel(const std::vector<const RDKit::ROMol*>& mo

#pragma omp for schedule(static)
for (int i = 0; i < numMols; ++i) {
const int molIdx = useSortOrder ? sortOrder[i] : i;
addQueryToBatch(molecules[molIdx], localBatch);
try {
const int molIdx = useSortOrder ? sortOrder[i] : i;
addQueryToBatch(molecules[molIdx], localBatch);
} catch (...) {
exceptionRegistry.store(std::current_exception());
}
}
}
exceptionRegistry.rethrow();

MoleculesHost result;
{
Expand Down
Loading