Fix some more mem errors in 2024.09.1 (#7899)

* fixes

* do not leak MolCatalogParams

* do not leak points on align failures

* give python ownership of pointers returned in getFingerprintsHelper

* clean up ScaffoldNetwork ptr if createNetworkHelper fails

* manage FF ptrs during construction

* wire in ownsBondInvGenerator in getMorganGenerator

* manage weights in rdMolAlign CalcRMS

* fix ownership of matches list/tuple in generateRmsdTransMatchPyTuple

* manage stream in createForwardSupplier during construction

* drop redundant Point3D allocations in GetUSRDistributionsFromPoints

* fix signed comparison mismatch
This commit is contained in:
Ricardo Rodriguez
2024-10-10 10:08:50 -04:00
committed by greg landrum
parent 156289c500
commit 518044f574
16 changed files with 103 additions and 82 deletions

View File

@@ -100,7 +100,7 @@ PyObject *embedBoundsMatrix(python::object boundsMatArg, int maxIters = 10,
DistGeom::BoundsMatrix::DATA_SPTR sdata(cData);
DistGeom::BoundsMatrix bm(nrows, sdata);
auto *positions = new RDGeom::Point3D[nrows];
std::unique_ptr<RDGeom::Point3D[]> positions(new RDGeom::Point3D[nrows]);
std::vector<RDGeom::Point *> posPtrs;
for (unsigned int i = 0; i < nrows; i++) {
posPtrs.push_back(&positions[i]);
@@ -167,7 +167,6 @@ PyObject *embedBoundsMatrix(python::object boundsMatArg, int maxIters = 10,
resData[iTab + j] = positions[i][j]; //.x;
}
}
delete[] positions;
return PyArray_Return(res);
}

View File

@@ -194,8 +194,11 @@ python::tuple PyForceField::minimizeTrajectory(unsigned int snapshotFreq,
int resInt = this->field->minimize(snapshotFreq, &snapshotVect, maxIts,
forceTol, energyTol);
python::list l;
boost::python::manage_new_object::apply<RDKit::Snapshot *>::type converter;
for (const auto &it : snapshotVect) {
l.append(new RDKit::Snapshot(it));
// transfer ownership to python
python::handle<> handle(converter(new RDKit::Snapshot(it)));
l.append(handle);
}
return python::make_tuple(resInt, l);
}

View File

@@ -652,9 +652,7 @@ python::list GetUSRDistributionsFromPoints(python::object coords,
}
RDGeom::Point3DConstPtrVect c(numCoords);
for (unsigned int i = 0; i < numCoords; ++i) {
auto *pt = new RDGeom::Point3D;
*pt = python::extract<RDGeom::Point3D>(coords[i]);
c[i] = pt;
c[i] = python::extract<RDGeom::Point3D *>(coords[i]);
}
std::vector<RDGeom::Point3D> p(numPts);
if (numPts == 0) {
@@ -673,9 +671,7 @@ python::list GetUSRDistributionsFromPoints(python::object coords,
}
pyDist.append(pytmp);
}
for (const auto *pt : c) {
delete pt;
}
return pyDist;
}

View File

@@ -46,12 +46,13 @@ void MultithreadedMolSupplier::writer() {
std::tuple<std::string, unsigned int, unsigned int> r;
while (d_inputQueue->pop(r)) {
try {
auto mol = processMoleculeRecord(std::get<0>(r), std::get<1>(r));
std::unique_ptr<RWMol> mol(
processMoleculeRecord(std::get<0>(r), std::get<1>(r)));
if (mol && writeCallback) {
writeCallback(*mol, std::get<0>(r), std::get<2>(r));
}
auto temp = std::tuple<RWMol *, std::string, unsigned int>{
mol, std::get<0>(r), std::get<2>(r)};
mol.release(), std::get<0>(r), std::get<2>(r)};
d_outputQueue->push(temp);
} catch (...) {
// fill the queue wih a null value

View File

@@ -172,7 +172,7 @@ MorganEnvGenerator<OutputType>::getEnvironments(
unsigned int nAtoms = mol.getNumAtoms();
auto *morganArguments = dynamic_cast<MorganArguments *>(arguments);
const unsigned int maxNumResults = (morganArguments->d_radius + 1) * nAtoms;
const unsigned int maxNumResults = (morganArguments->d_radius + 1) * nAtoms;
std::vector<AtomEnvironment<OutputType> *> result =
std::vector<AtomEnvironment<OutputType> *>();
@@ -267,7 +267,8 @@ MorganEnvGenerator<OutputType>::getEnvironments(
boost::tie(beg, end) = mol.getAtomBonds(tAtom);
// add up to date invariants of neighbors
// This should keep capacity, so reallocation only triggers if we haven't seen a molecule of this size.
// This should keep capacity, so reallocation only triggers if we
// haven't seen a molecule of this size.
neighborhoodInvariants.clear();
while (beg != end) {
@@ -384,8 +385,7 @@ FingerprintGenerator<OutputType> *getMorganGenerator(
AtomInvariantsGenerator *atomInvariantsGenerator,
BondInvariantsGenerator *bondInvariantsGenerator, std::uint32_t fpSize,
std::vector<std::uint32_t> countBounds, bool ownsAtomInvGen,
bool // ownsBondInvGen
) {
bool ownsBondInvGen) {
AtomEnvironmentGenerator<OutputType> *morganEnvGenerator =
new MorganEnvGenerator<OutputType>();
FingerprintArguments *morganArguments = new MorganArguments(
@@ -398,7 +398,7 @@ FingerprintGenerator<OutputType> *getMorganGenerator(
ownsAtomInvGenerator = true;
}
bool ownsBondInvGenerator = false;
bool ownsBondInvGenerator = ownsBondInvGen;
if (!bondInvariantsGenerator) {
bondInvariantsGenerator =
new MorganBondInvGenerator(useBondTypes, includeChirality);

View File

@@ -976,7 +976,7 @@ ForceFields::ForceField *constructForceField(
PRECONDITION(mmffMolProperties->isValid(),
"missing atom types - invalid force-field");
auto *res = new ForceFields::ForceField();
std::unique_ptr<ForceFields::ForceField> res(new ForceFields::ForceField());
// add the atomic positions:
Conformer &conf = mol.getConformer(confId);
for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) {
@@ -985,35 +985,35 @@ ForceFields::ForceField *constructForceField(
res->initialize();
if (mmffMolProperties->getMMFFBondTerm()) {
Tools::addBonds(mol, mmffMolProperties, res);
Tools::addBonds(mol, mmffMolProperties, res.get());
}
if (mmffMolProperties->getMMFFAngleTerm()) {
Tools::addAngles(mol, mmffMolProperties, res);
Tools::addAngles(mol, mmffMolProperties, res.get());
}
if (mmffMolProperties->getMMFFStretchBendTerm()) {
Tools::addStretchBend(mol, mmffMolProperties, res);
Tools::addStretchBend(mol, mmffMolProperties, res.get());
}
if (mmffMolProperties->getMMFFOopTerm()) {
Tools::addOop(mol, mmffMolProperties, res);
Tools::addOop(mol, mmffMolProperties, res.get());
}
if (mmffMolProperties->getMMFFTorsionTerm()) {
Tools::addTorsions(mol, mmffMolProperties, res);
Tools::addTorsions(mol, mmffMolProperties, res.get());
}
if (mmffMolProperties->getMMFFVdWTerm() ||
mmffMolProperties->getMMFFEleTerm()) {
boost::shared_array<std::uint8_t> neighborMat =
Tools::buildNeighborMatrix(mol);
if (mmffMolProperties->getMMFFVdWTerm()) {
Tools::addVdW(mol, confId, mmffMolProperties, res, neighborMat,
Tools::addVdW(mol, confId, mmffMolProperties, res.get(), neighborMat,
nonBondedThresh, ignoreInterfragInteractions);
}
if (mmffMolProperties->getMMFFEleTerm()) {
Tools::addEle(mol, confId, mmffMolProperties, res, neighborMat,
Tools::addEle(mol, confId, mmffMolProperties, res.get(), neighborMat,
nonBondedThresh, ignoreInterfragInteractions);
}
}
return res;
return res.release();
}
} // namespace MMFF
} // namespace RDKit

View File

@@ -49,10 +49,9 @@ inline std::pair<int, double> MMFFOptimizeMolecule(
std::pair<int, double> res = std::make_pair(-1, -1);
MMFF::MMFFMolProperties mmffMolProperties(mol, mmffVariant);
if (mmffMolProperties.isValid()) {
ForceFields::ForceField *ff = MMFF::constructForceField(
mol, nonBondedThresh, confId, ignoreInterfragInteractions);
std::unique_ptr<ForceFields::ForceField> ff(MMFF::constructForceField(
mol, nonBondedThresh, confId, ignoreInterfragInteractions));
res = ForceFieldsHelper::OptimizeMolecule(*ff, maxIters);
delete ff;
}
return res;
}
@@ -87,12 +86,11 @@ inline void MMFFOptimizeMoleculeConfs(ROMol &mol,
bool ignoreInterfragInteractions = true) {
MMFF::MMFFMolProperties mmffMolProperties(mol, mmffVariant);
if (mmffMolProperties.isValid()) {
ForceFields::ForceField *ff =
std::unique_ptr<ForceFields::ForceField> ff(
MMFF::constructForceField(mol, &mmffMolProperties, nonBondedThresh, -1,
ignoreInterfragInteractions);
ignoreInterfragInteractions));
ForceFieldsHelper::OptimizeMoleculeConfs(mol, *ff, res, numThreads,
maxIters);
delete ff;
} else {
res.resize(mol.getNumConformers());
for (unsigned int i = 0; i < mol.getNumConformers(); ++i) {

View File

@@ -705,7 +705,7 @@ ForceFields::ForceField *constructForceField(ROMol &mol,
<< std::endl;
}
auto *res = new ForceFields::ForceField();
std::unique_ptr<ForceFields::ForceField> res(new ForceFields::ForceField());
// add the atomic positions:
Conformer &conf = mol.getConformer(confId);
@@ -713,17 +713,17 @@ ForceFields::ForceField *constructForceField(ROMol &mol,
res->positions().push_back(&conf.getAtomPos(i));
}
Tools::addBonds(mol, params, res);
Tools::addAngles(mol, params, res);
Tools::addAngleSpecialCases(mol, confId, params, res);
Tools::addBonds(mol, params, res.get());
Tools::addAngles(mol, params, res.get());
Tools::addAngleSpecialCases(mol, confId, params, res.get());
boost::shared_array<std::uint8_t> neighborMat =
Tools::buildNeighborMatrix(mol);
Tools::addNonbonded(mol, confId, params, res, neighborMat, vdwThresh,
Tools::addNonbonded(mol, confId, params, res.get(), neighborMat, vdwThresh,
ignoreInterfragInteractions);
Tools::addTorsions(mol, params, res);
Tools::addInversions(mol, params, res);
Tools::addTorsions(mol, params, res.get());
Tools::addInversions(mol, params, res.get());
return res;
return res.release();
}
// ------------------------------------------------------------------------

View File

@@ -40,11 +40,10 @@ namespace UFF {
inline std::pair<int, double> UFFOptimizeMolecule(
ROMol &mol, int maxIters = 1000, double vdwThresh = 10.0, int confId = -1,
bool ignoreInterfragInteractions = true) {
ForceFields::ForceField *ff = UFF::constructForceField(
mol, vdwThresh, confId, ignoreInterfragInteractions);
std::unique_ptr<ForceFields::ForceField> ff(UFF::constructForceField(
mol, vdwThresh, confId, ignoreInterfragInteractions));
std::pair<int, double> res =
ForceFieldsHelper::OptimizeMolecule(*ff, maxIters);
delete ff;
return res;
}
@@ -72,10 +71,9 @@ inline void UFFOptimizeMoleculeConfs(ROMol &mol,
int numThreads = 1, int maxIters = 1000,
double vdwThresh = 10.0,
bool ignoreInterfragInteractions = true) {
ForceFields::ForceField *ff =
UFF::constructForceField(mol, vdwThresh, -1, ignoreInterfragInteractions);
std::unique_ptr<ForceFields::ForceField> ff(UFF::constructForceField(
mol, vdwThresh, -1, ignoreInterfragInteractions));
ForceFieldsHelper::OptimizeMoleculeConfs(mol, *ff, res, numThreads, maxIters);
delete ff;
}
} // end of namespace UFF
} // end of namespace RDKit

View File

@@ -120,12 +120,11 @@ int MMFFOptimizeMolecule(ROMol &mol, std::string mmffVariant = "MMFF94",
MMFF::MMFFMolProperties mmffMolProperties(mol, mmffVariant);
if (mmffMolProperties.isValid()) {
NOGIL gil;
ForceFields::ForceField *ff =
std::unique_ptr<ForceFields::ForceField> ff(
MMFF::constructForceField(mol, &mmffMolProperties, nonBondedThresh,
confId, ignoreInterfragInteractions);
confId, ignoreInterfragInteractions));
ff->initialize();
res = ff->minimize(maxIters);
delete ff;
}
return res;

View File

@@ -72,12 +72,15 @@ PyObject *generateRmsdTransMatchPyTuple(double rmsd,
PyTuple_SetItem(resTup, 0, rmsdItem);
PyTuple_SetItem(resTup, 1, PyArray_Return(res));
if (match) {
python::list pairList;
for (const auto &pair : *match) {
pairList.append(python::make_tuple(pair.first, pair.second));
PyObject *listTup = PyTuple_New(match->size());
for (unsigned int i = 0; i < match->size(); ++i) {
auto *pairTup = PyTuple_New(2);
PyTuple_SetItem(pairTup, 0, PyLong_FromLong((*match)[i].first));
PyTuple_SetItem(pairTup, 1, PyLong_FromLong((*match)[i].second));
PyTuple_SetItem(listTup, i, pairTup);
}
auto pairTup = new python::tuple(pairList);
PyTuple_SetItem(resTup, 2, pairTup->ptr());
PyTuple_SetItem(resTup, 2, listTup);
}
return resTup;
}
@@ -224,12 +227,13 @@ double CalcRMS(ROMol &prbMol, ROMol &refMol, int prbCid, int refCid,
if (map != python::object()) {
aMapVec = translateAtomMapSeq(map);
}
RDNumeric::DoubleVector *wtsVec = translateDoubleSeq(weights);
std::unique_ptr<RDNumeric::DoubleVector> wtsVec(translateDoubleSeq(weights));
double rmsd;
{
NOGIL gil;
rmsd = MolAlign::CalcRMS(prbMol, refMol, prbCid, refCid, aMapVec,
maxMatches, symmetrizeTerminalGroups, wtsVec);
rmsd =
MolAlign::CalcRMS(prbMol, refMol, prbCid, refCid, aMapVec, maxMatches,
symmetrizeTerminalGroups, wtsVec.get());
}
return rmsd;
}
@@ -406,8 +410,11 @@ python::tuple getMMFFO3AForConfs(
}
python::list pyres;
boost::python::manage_new_object::apply<PyO3A *>::type converter;
for (auto &i : res) {
pyres.append(new PyO3A(i));
// transfer ownership to python
python::handle<> handle(converter(new PyO3A(i)));
pyres.append(handle);
}
return python::tuple(pyres);
@@ -562,9 +569,13 @@ python::tuple getCrippenO3AForConfs(
numThreads, MolAlign::O3A::CRIPPEN, refCid, reflect,
maxIters, options, cMap.get(), cWts.get());
}
python::list pyres;
for (auto &re : res) {
pyres.append(new PyO3A(re));
boost::python::manage_new_object::apply<PyO3A *>::type converter;
for (auto &i : res) {
// transfer ownership to python
python::handle<> handle(converter(new PyO3A(i)));
pyres.append(handle);
}
return python::tuple(pyres);

View File

@@ -78,7 +78,8 @@ const ROMol &catalogEntryGetMol(MolCatalogEntry &self) {
}
MolCatalog *createMolCatalog() {
return new MolCatalog(new MolCatalogParams());
MolCatalogParams params;
return new MolCatalog(&params);
}
struct MolCatalog_wrapper {
static void wrap() {

View File

@@ -34,8 +34,11 @@ python::tuple getFingerprintsHelper(
generator);
}
python::list pyFingerprints;
boost::python::manage_new_object::apply<ExplicitBitVect *>::type converter;
for (auto &fp : fps) {
pyFingerprints.append(fp.release());
// transfer ownership to python
python::handle<> handle(converter(fp.release()));
pyFingerprints.append(handle);
}
return python::tuple(pyFingerprints);

View File

@@ -22,13 +22,14 @@ ScaffoldNetwork::ScaffoldNetwork *createNetworkHelper(
python::object pmols,
const ScaffoldNetwork::ScaffoldNetworkParams &params) {
auto mols = pythonObjectToVect<ROMOL_SPTR>(pmols);
ScaffoldNetwork::ScaffoldNetwork *res = new ScaffoldNetwork::ScaffoldNetwork;
std::unique_ptr<ScaffoldNetwork::ScaffoldNetwork> res(
new ScaffoldNetwork::ScaffoldNetwork);
if (mols) {
NOGIL gil;
updateScaffoldNetwork(*mols, *res, params);
}
return res;
return res.release();
}
void updateNetworkHelper(python::object pmols,
ScaffoldNetwork::ScaffoldNetwork &net,

View File

@@ -62,7 +62,7 @@ ForwardSDMolSupplier *createForwardSupplier(std::string filename, bool sanitize,
bool removeHs) {
std::vector<std::string> splitName;
boost::split(splitName, filename, boost::is_any_of("."));
io::filtering_istream *strm = new io::filtering_istream();
std::unique_ptr<io::filtering_istream> strm(new io::filtering_istream());
if (splitName.back() == "sdf") {
} else if (splitName.back() == "gz") {
#ifndef RDK_NOGZIP
@@ -88,7 +88,7 @@ ForwardSDMolSupplier *createForwardSupplier(std::string filename, bool sanitize,
strm->push(fileSource);
ForwardSDMolSupplier *res =
new ForwardSDMolSupplier(strm, true, sanitize, removeHs);
new ForwardSDMolSupplier(strm.release(), true, sanitize, removeHs);
return res;
}

View File

@@ -23,6 +23,24 @@ namespace python = boost::python;
namespace RDNumeric {
namespace Alignments {
class PointVectManager {
public:
PointVectManager() = default;
PointVectManager(const PointVectManager &rhs) = delete;
PointVectManager &operator=(const PointVectManager &rhs) = delete;
~PointVectManager() {
for (auto &i : m_ptr) {
delete i;
}
}
RDGeom::Point3DConstPtrVect &getVect() { return m_ptr; }
private:
RDGeom::Point3DConstPtrVect m_ptr;
};
void GetPointsFromPythonSequence(python::object &points,
RDGeom::Point3DConstPtrVect &pts) {
PyObject *pyObj = points.ptr();
@@ -89,19 +107,18 @@ PyObject *AlignPointPairs(python::object refPoints, python::object probePoints,
// 2. A list of doubles of size N
// first deal with situation where we have Numerics arrays
RDGeom::Point3DConstPtrVect refPts, probePts;
PointVectManager refPts, probePts;
GetPointsFromPythonSequence(refPoints, refPts);
GetPointsFromPythonSequence(probePoints, probePts);
GetPointsFromPythonSequence(refPoints, refPts.getVect());
GetPointsFromPythonSequence(probePoints, probePts.getVect());
unsigned int npt = refPts.size();
if (npt != probePts.size()) {
unsigned int npt = refPts.getVect().size();
if (npt != probePts.getVect().size()) {
throw_value_error("Mis-match in number of points");
}
PyObject *weightsObj = weights.ptr();
RDNumeric::DoubleVector *wtsVec;
wtsVec = nullptr;
std::unique_ptr<RDNumeric::DoubleVector> wtsVec;
double *data;
if (PyArray_Check(weightsObj)) {
auto *wtsMat = reinterpret_cast<PyArrayObject *>(weightsObj);
@@ -110,7 +127,7 @@ PyObject *AlignPointPairs(python::object refPoints, python::object probePoints,
throw_value_error(
"Number of weights supplied do not match the number of points");
}
wtsVec = new RDNumeric::DoubleVector(nwts);
wtsVec.reset(new RDNumeric::DoubleVector(nwts));
data = reinterpret_cast<double *>(PyArray_DATA(wtsMat));
for (unsigned int i = 0; i < nwts; i++) {
wtsVec->setVal(i, data[i]);
@@ -124,7 +141,7 @@ PyObject *AlignPointPairs(python::object refPoints, python::object probePoints,
throw_value_error(
"Number of weights supplied do not match the number of points");
}
wtsVec = new RDNumeric::DoubleVector(nwts);
wtsVec.reset(new RDNumeric::DoubleVector(nwts));
for (unsigned int i = 0; i < npt; i++) {
wtsVec->setVal(i, wts[i]);
}
@@ -132,8 +149,8 @@ PyObject *AlignPointPairs(python::object refPoints, python::object probePoints,
}
RDGeom::Transform3D trans;
double ssd =
AlignPoints(refPts, probePts, trans, wtsVec, reflect, maxIterations);
double ssd = AlignPoints(refPts.getVect(), probePts.getVect(), trans,
wtsVec.get(), reflect, maxIterations);
npy_intp dims[2];
dims[0] = 4;
@@ -148,12 +165,6 @@ PyObject *AlignPointPairs(python::object refPoints, python::object probePoints,
}
}
delete wtsVec;
for (unsigned int i = 0; i < npt; ++i) {
delete probePts[i];
delete refPts[i];
}
PyObject *resTup = PyTuple_New(2);
PyObject *ssdItem = PyFloat_FromDouble(ssd);
PyTuple_SetItem(resTup, 0, ssdItem);