diff --git a/Code/GraphMol/hanoitest.cpp b/Code/GraphMol/hanoitest.cpp index 9ef909f80..f6dec3f1e 100644 --- a/Code/GraphMol/hanoitest.cpp +++ b/Code/GraphMol/hanoitest.cpp @@ -1,5 +1,5 @@ // -// Copyright (C) 2014 Greg Landrum +// Copyright (C) 2014-2025 Greg Landrum and other RDKit contributors // // @@ All Rights Reserved @@ // This file is part of the RDKit. @@ -18,9 +18,12 @@ #include #include +#include #include #include #include +#include +#include using namespace RDKit; @@ -64,7 +67,7 @@ class int_compare_ftor { void qs1(const std::vector> &vects) { BOOST_LOG(rdInfoLog) << "sorting (qsort) vectors" << std::endl; for (auto tv : vects) { - int *data = &tv.front(); + auto *data = tv.data(); qsort(data, tv.size(), sizeof(int), pcmp); for (unsigned int j = 1; j < tv.size(); ++j) { TEST_ASSERT(tv[j] >= tv[j - 1]); @@ -76,22 +79,19 @@ void qs1(const std::vector> &vects) { void hs1(const std::vector> &vects) { BOOST_LOG(rdInfoLog) << "sorting (hanoi sort) vectors" << std::endl; for (const auto &vect : vects) { - const int *data = &vect.front(); + const int *data = vect.data(); int_compare_ftor icmp(data); - int *indices = (int *)malloc(vect.size() * sizeof(int)); + std::vector indices(vect.size()); + std::span ispan(indices); for (unsigned int j = 0; j < vect.size(); ++j) { indices[j] = j; } - int *count = (int *)malloc(vect.size() * sizeof(int)); - int *changed = (int *)malloc(vect.size() * sizeof(int)); - memset(changed, 1, vect.size() * sizeof(int)); - RDKit::hanoisort(indices, vect.size(), count, changed, icmp); + std::vector count(vect.size()); + std::vector changed(vect.size(), 1); + RDKit::hanoisort(ispan, count, changed, icmp); for (unsigned int j = 1; j < vect.size(); ++j) { TEST_ASSERT(data[indices[j]] >= data[indices[j - 1]]); } - free(count); - free(indices); - free(changed); } BOOST_LOG(rdInfoLog) << "done: " << vects.size() << std::endl; } @@ -204,7 +204,7 @@ void test2() { // make sure that hanoi works with a functor and "molecule data" { std::string smi = "FC1C(Cl)C1C"; - RWMol *m = SmilesToMol(smi); + auto m = v2::SmilesParse::MolFromSmiles(smi); TEST_ASSERT(m); std::vector atoms(m->getNumAtoms()); std::vector indices(m->getNumAtoms()); @@ -213,13 +213,12 @@ void test2() { atoms[i].index = 0; indices[i] = i; } - atomcomparefunctor ftor(&atoms.front()); + atomcomparefunctor ftor(atoms.data()); - int *data = &indices.front(); - int *count = (int *)malloc(atoms.size() * sizeof(int)); - int *changed = (int *)malloc(atoms.size() * sizeof(int)); - memset(changed, 1, atoms.size() * sizeof(int)); - RDKit::hanoisort(data, atoms.size(), count, changed, ftor); + std::span ispan(indices); + std::vector count(atoms.size()); + std::vector changed(atoms.size(), 1); + RDKit::hanoisort(ispan, count, changed, ftor); for (unsigned int i = 0; i < m->getNumAtoms(); ++i) { // std::cerr< atoms(m->getNumAtoms()); initCanonAtoms(*m, atoms, true); - atomcomparefunctor ftor(&atoms.front()); + atomcomparefunctor ftor(atoms.data()); - RDKit::Canon::canon_atom *data = &atoms.front(); - int *count = (int *)malloc(atoms.size() * sizeof(int)); - int *order = (int *)malloc(atoms.size() * sizeof(int)); + auto *data = atoms.data(); + std::vector count(atoms.size()); + std::vector order(atoms.size()); int activeset; - int *next = (int *)malloc(atoms.size() * sizeof(int)); - int *changed = (int *)malloc(atoms.size() * sizeof(int)); - memset(changed, 1, atoms.size() * sizeof(int)); - char *touched = (char *)malloc(atoms.size() * sizeof(char)); - memset(touched, 0, atoms.size() * sizeof(char)); + std::vector next(atoms.size()); + std::vector changed(atoms.size(), 1); + std::vector touched(atoms.size(), 0); RDKit::Canon::CreateSinglePartition(atoms.size(), order, count, data); RDKit::Canon::ActivatePartitions(atoms.size(), order, count, activeset, @@ -299,11 +293,6 @@ void test3() { TEST_ASSERT(count[order[7]] == 1); delete m; - free(count); - free(order); - free(next); - free(changed); - free(touched); } { // this time with smarter invariants @@ -312,17 +301,16 @@ void test3() { TEST_ASSERT(m); std::vector atoms(m->getNumAtoms()); initCanonAtoms(*m, atoms, true); - atomcomparefunctor2 ftor(&atoms.front()); + atomcomparefunctor2 ftor(atoms.data()); - RDKit::Canon::canon_atom *data = &atoms.front(); - int *count = (int *)malloc(atoms.size() * sizeof(int)); - int *order = (int *)malloc(atoms.size() * sizeof(int)); + auto *data = atoms.data(); + std::vector count(atoms.size()); + + std::vector order(atoms.size()); int activeset; - int *next = (int *)malloc(atoms.size() * sizeof(int)); - int *changed = (int *)malloc(atoms.size() * sizeof(int)); - memset(changed, 1, atoms.size() * sizeof(int)); - char *touched = (char *)malloc(atoms.size() * sizeof(char)); - memset(touched, 0, atoms.size() * sizeof(char)); + std::vector next(atoms.size()); + std::vector changed(atoms.size(), 1); + std::vector touched(atoms.size(), 0); RDKit::Canon::CreateSinglePartition(atoms.size(), order, count, data); RDKit::Canon::ActivatePartitions(atoms.size(), order, count, activeset, @@ -350,11 +338,6 @@ void test3() { TEST_ASSERT(count[order[5]] == 3); TEST_ASSERT(count[order[6]] == 0); delete m; - free(count); - free(order); - free(next); - free(changed); - free(touched); } BOOST_LOG(rdInfoLog) << "Done" << std::endl; }; @@ -455,16 +438,14 @@ void test4() { TEST_ASSERT(m); std::vector atoms(m->getNumAtoms()); initCanonAtoms(*m, atoms, true); - atomcomparefunctor3 ftor(&atoms.front(), *m); - RDKit::Canon::canon_atom *data = &atoms.front(); - int *count = (int *)malloc(atoms.size() * sizeof(int)); - int *order = (int *)malloc(atoms.size() * sizeof(int)); + atomcomparefunctor3 ftor(atoms.data(), *m); + auto *data = atoms.data(); + std::vector count(atoms.size()); + std::vector order(atoms.size()); int activeset; - int *next = (int *)malloc(atoms.size() * sizeof(int)); - int *changed = (int *)malloc(atoms.size() * sizeof(int)); - memset(changed, 1, atoms.size() * sizeof(int)); - char *touched = (char *)malloc(atoms.size() * sizeof(char)); - memset(touched, 0, atoms.size() * sizeof(char)); + std::vector next(atoms.size()); + std::vector changed(atoms.size(), 1); + std::vector touched(atoms.size(), 0); RDKit::Canon::CreateSinglePartition(atoms.size(), order, count, data); RDKit::Canon::ActivatePartitions(atoms.size(), order, count, activeset, @@ -508,11 +489,6 @@ void test4() { } } delete m; - free(count); - free(order); - free(next); - free(changed); - free(touched); } { @@ -521,17 +497,15 @@ void test4() { TEST_ASSERT(m); std::vector atoms(m->getNumAtoms()); initCanonAtoms(*m, atoms, true); - atomcomparefunctor3 ftor(&atoms.front(), *m); + atomcomparefunctor3 ftor(atoms.data(), *m); - RDKit::Canon::canon_atom *data = &atoms.front(); - int *count = (int *)malloc(atoms.size() * sizeof(int)); - int *order = (int *)malloc(atoms.size() * sizeof(int)); + auto data = atoms.data(); + std::vector count(atoms.size()); + std::vector order(atoms.size()); int activeset; - int *next = (int *)malloc(atoms.size() * sizeof(int)); - int *changed = (int *)malloc(atoms.size() * sizeof(int)); - memset(changed, 1, atoms.size() * sizeof(int)); - char *touched = (char *)malloc(atoms.size() * sizeof(char)); - memset(touched, 0, atoms.size() * sizeof(char)); + std::vector next(atoms.size()); + std::vector changed(atoms.size(), 1); + std::vector touched(atoms.size(), 0); RDKit::Canon::CreateSinglePartition(atoms.size(), order, count, data); RDKit::Canon::ActivatePartitions(atoms.size(), order, count, activeset, @@ -562,11 +536,6 @@ void test4() { } } delete m; - free(count); - free(order); - free(next); - free(changed); - free(touched); } { @@ -575,17 +544,15 @@ void test4() { TEST_ASSERT(m); std::vector atoms(m->getNumAtoms()); initCanonAtoms(*m, atoms, true); - atomcomparefunctor3 ftor(&atoms.front(), *m); + atomcomparefunctor3 ftor(atoms.data(), *m); - RDKit::Canon::canon_atom *data = &atoms.front(); - int *count = (int *)malloc(atoms.size() * sizeof(int)); - int *order = (int *)malloc(atoms.size() * sizeof(int)); + auto *data = atoms.data(); + std::vector count(atoms.size()); + std::vector order(atoms.size()); int activeset; - int *next = (int *)malloc(atoms.size() * sizeof(int)); - int *changed = (int *)malloc(atoms.size() * sizeof(int)); - memset(changed, 1, atoms.size() * sizeof(int)); - char *touched = (char *)malloc(atoms.size() * sizeof(char)); - memset(touched, 0, atoms.size() * sizeof(char)); + std::vector next(atoms.size()); + std::vector changed(atoms.size(), 1); + std::vector touched(atoms.size(), 0); RDKit::Canon::CreateSinglePartition(atoms.size(), order, count, data); RDKit::Canon::ActivatePartitions(atoms.size(), order, count, activeset, @@ -631,11 +598,6 @@ void test4() { TEST_ASSERT(order[9] == 1 && count[1] == 1); delete m; - free(count); - free(order); - free(next); - free(changed); - free(touched); } BOOST_LOG(rdInfoLog) << "Done" << std::endl; @@ -651,17 +613,15 @@ void test5() { TEST_ASSERT(m); std::vector atoms(m->getNumAtoms()); initCanonAtoms(*m, atoms, true); - atomcomparefunctor3 ftor(&atoms.front(), *m); + atomcomparefunctor3 ftor(atoms.data(), *m); - RDKit::Canon::canon_atom *data = &atoms.front(); - int *count = (int *)malloc(atoms.size() * sizeof(int)); - int *order = (int *)malloc(atoms.size() * sizeof(int)); + auto *data = atoms.data(); + std::vector count(atoms.size()); + std::vector order(atoms.size()); int activeset; - int *next = (int *)malloc(atoms.size() * sizeof(int)); - int *changed = (int *)malloc(atoms.size() * sizeof(int)); - memset(changed, 1, atoms.size() * sizeof(int)); - char *touched = (char *)malloc(atoms.size() * sizeof(char)); - memset(touched, 0, atoms.size() * sizeof(char)); + std::vector next(atoms.size()); + std::vector changed(atoms.size(), 1); + std::vector touched(atoms.size(), 0); RDKit::Canon::CreateSinglePartition(atoms.size(), order, count, data); RDKit::Canon::ActivatePartitions(atoms.size(), order, count, activeset, @@ -709,11 +669,6 @@ void test5() { TEST_ASSERT(count[order[i]] == 1); } delete m; - free(count); - free(order); - free(next); - free(changed); - free(touched); } BOOST_LOG(rdInfoLog) << "Done" << std::endl; }; @@ -1154,14 +1109,21 @@ std::string smis[] = { void test7() { BOOST_LOG(rdInfoLog) << "testing stability w.r.t. renumbering." << std::endl; unsigned int i = 0; + auto start = std::chrono::high_resolution_clock::now(); + while (smis[i] != "EOS") { std::string smiles = smis[i++]; ROMol *m = SmilesToMol(smiles); TEST_ASSERT(m); MolOps::assignStereochemistry(*m, true); - //_renumberTest(m, smiles, 1000); + _renumberTest(m, smiles, 1000); delete m; } + auto end = std::chrono::high_resolution_clock::now(); + auto diff = + std::chrono::duration_cast(end - start); + BOOST_LOG(rdInfoLog) << " Finished in " << diff.count() << " ms" + << std::endl; BOOST_LOG(rdInfoLog) << "Finished" << std::endl; } diff --git a/Code/GraphMol/new_canon.cpp b/Code/GraphMol/new_canon.cpp index 319b69997..d05b2048b 100644 --- a/Code/GraphMol/new_canon.cpp +++ b/Code/GraphMol/new_canon.cpp @@ -13,9 +13,10 @@ #include #include #include -#include +#include #include #include +#include // #define VERBOSE_CANON 1 namespace RDKit { @@ -92,10 +93,10 @@ int bondholder::compareStereo(const bondholder &o) const { return 0; } -void CreateSinglePartition(unsigned int nAtoms, int *order, int *count, - canon_atom *atoms) { - PRECONDITION(order, "bad pointer"); - PRECONDITION(count, "bad pointer"); +void CreateSinglePartition(unsigned int nAtoms, std::vector &order, + std::vector &count, canon_atom *atoms) { + PRECONDITION(!order.empty(), "order should not be empty"); + PRECONDITION(!count.empty(), "count should not be empty"); PRECONDITION(atoms, "bad pointer"); for (unsigned int i = 0; i < nAtoms; i++) { @@ -106,12 +107,13 @@ void CreateSinglePartition(unsigned int nAtoms, int *order, int *count, count[0] = nAtoms; } -void ActivatePartitions(unsigned int nAtoms, int *order, int *count, - int &activeset, int *next, int *changed) { - PRECONDITION(order, "bad pointer"); - PRECONDITION(count, "bad pointer"); - PRECONDITION(next, "bad pointer"); - PRECONDITION(changed, "bad pointer"); +void ActivatePartitions(unsigned int nAtoms, std::vector &order, + std::vector &count, int &activeset, + std::vector &next, std::vector &changed) { + PRECONDITION(!order.empty(), "order should not be empty"); + PRECONDITION(!count.empty(), "count should not be empty"); + PRECONDITION(!next.empty(), "next should not be empty"); + PRECONDITION(!changed.empty(), "changed should not be empty"); unsigned int i, j; activeset = -1; for (i = 0; i < nAtoms; i++) { @@ -150,10 +152,10 @@ void compareRingAtomsConcerningNumNeighbors(Canon::canon_atom *atoms, PRECONDITION(atoms, "bad pointer"); RingInfo *ringInfo = mol.getRingInfo(); - auto visited = std::make_unique(nAtoms); - auto lastLevelNbrs = std::make_unique(nAtoms); - auto currentLevelNbrs = std::make_unique(nAtoms); - auto revisitedNeighbors = std::make_unique(nAtoms); + std::vector visited(nAtoms); + std::vector lastLevelNbrs(nAtoms); + std::vector currentLevelNbrs(nAtoms); + std::vector revisitedNeighbors(nAtoms); for (unsigned idx = 0; idx < nAtoms; ++idx) { const Canon::canon_atom &a = atoms[idx]; if (!ringInfo->isInitialized() || @@ -166,10 +168,10 @@ void compareRingAtomsConcerningNumNeighbors(Canon::canon_atom *atoms, atoms[idx].neighborNum.reserve(1000); atoms[idx].revistedNeighbors.assign(1000, 0); - memset(visited.get(), 0, nAtoms * sizeof(char)); - memset(lastLevelNbrs.get(), 0, nAtoms * sizeof(char)); - memset(currentLevelNbrs.get(), 0, nAtoms * sizeof(char)); - memset(revisitedNeighbors.get(), 0, nAtoms * sizeof(int)); + std::fill(visited.begin(), visited.end(), 0); + std::fill(lastLevelNbrs.begin(), lastLevelNbrs.end(), 0); + std::fill(currentLevelNbrs.begin(), currentLevelNbrs.end(), 0); + std::fill(revisitedNeighbors.begin(), revisitedNeighbors.end(), 0); std::vector nextLevelNbrs; while (!neighbors.empty()) { unsigned int numLevelNbrs = 0; @@ -205,13 +207,13 @@ void compareRingAtomsConcerningNumNeighbors(Canon::canon_atom *atoms, } } } - memset(lastLevelNbrs.get(), 0, nAtoms * sizeof(char)); + std::fill(lastLevelNbrs.begin(), lastLevelNbrs.end(), 0); for (unsigned i = 0; i < nAtoms; ++i) { if (currentLevelNbrs[i]) { lastLevelNbrs[i] = 1; } } - memset(currentLevelNbrs.get(), 0, nAtoms * sizeof(char)); + std::fill(currentLevelNbrs.begin(), currentLevelNbrs.end(), 0); std::vector tmp; tmp.reserve(30); for (unsigned i = 0; i < nAtoms; ++i) { @@ -229,7 +231,7 @@ void compareRingAtomsConcerningNumNeighbors(Canon::canon_atom *atoms, atoms[idx].revistedNeighbors[currentRNIdx] = i; currentRNIdx++; } - memset(revisitedNeighbors.get(), 0, nAtoms * sizeof(int)); + std::fill(revisitedNeighbors.begin(), revisitedNeighbors.end(), 0); atoms[idx].neighborNum.push_back(numLevelNbrs); atoms[idx].neighborNum.push_back(-1); @@ -243,25 +245,21 @@ void compareRingAtomsConcerningNumNeighbors(Canon::canon_atom *atoms, namespace detail { template -void rankWithFunctor(T &ftor, bool breakTies, int *order, bool useSpecial, - bool useChirality, bool includeRingStereo, +void rankWithFunctor(T &ftor, bool breakTies, std::vector &order, + bool useSpecial, bool useChirality, bool includeRingStereo, const boost::dynamic_bitset<> *atomsInPlay, const boost::dynamic_bitset<> *bondsInPlay) { - PRECONDITION(order, "bad pointer"); + PRECONDITION(!order.empty(), "order should not be empty"); const ROMol &mol = *ftor.dp_mol; canon_atom *atoms = ftor.dp_atoms; - unsigned int nAts = mol.getNumAtoms(); + const unsigned int nAts = mol.getNumAtoms(); - // auto order = std::make_unique(mol.getNumAtoms()); - - auto count = std::make_unique(nAts); - auto next = std::make_unique(nAts); - auto changed = std::make_unique(nAts); - memset(changed.get(), 1, nAts * sizeof(int)); - auto touched = std::make_unique(nAts); - memset(touched.get(), 0, nAts * sizeof(char)); + std::vector count(nAts); + std::vector next(nAts); + std::vector changed(nAts, 1); + std::vector touched(nAts, 0); int activeset; - CreateSinglePartition(nAts, order, count.get(), atoms); + CreateSinglePartition(nAts, order, count, atoms); // ActivatePartitions(nAts,order,count,activeset,next,changed); // RefinePartitions(mol,atoms,ftor,false,order,count,activeset,next,changed,touched); #ifdef VERBOSE_CANON @@ -272,8 +270,7 @@ void rankWithFunctor(T &ftor, bool breakTies, int *order, bool useSpecial, } #endif ftor.df_useNbrs = true; - ActivatePartitions(nAts, order, count.get(), activeset, next.get(), - changed.get()); + ActivatePartitions(nAts, order, count, activeset, next, changed); #ifdef VERBOSE_CANON std::cerr << "1a--------" << std::endl; for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) { @@ -281,8 +278,8 @@ void rankWithFunctor(T &ftor, bool breakTies, int *order, bool useSpecial, << " count: " << count[order[i]] << std::endl; } #endif - RefinePartitions(mol, atoms, ftor, true, order, count.get(), activeset, - next.get(), changed.get(), touched.get()); + RefinePartitions(mol, atoms, ftor, true, order, count, activeset, next, + changed, touched); #ifdef VERBOSE_CANON std::cerr << "2--------" << std::endl; for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) { @@ -299,10 +296,9 @@ void rankWithFunctor(T &ftor, bool breakTies, int *order, bool useSpecial, if (useChirality && ties && includeRingStereo) { SpecialChiralityAtomCompareFunctor scftor(atoms, mol, atomsInPlay, bondsInPlay); - ActivatePartitions(nAts, order, count.get(), activeset, next.get(), - changed.get()); - RefinePartitions(mol, atoms, scftor, true, order, count.get(), activeset, - next.get(), changed.get(), touched.get()); + ActivatePartitions(nAts, order, count, activeset, next, changed); + RefinePartitions(mol, atoms, scftor, true, order, count, activeset, next, + changed, touched); #ifdef VERBOSE_CANON std::cerr << "2a--------" << std::endl; for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) { @@ -337,10 +333,9 @@ void rankWithFunctor(T &ftor, bool breakTies, int *order, bool useSpecial, SpecialSymmetryAtomCompareFunctor sftor(atoms, mol, atomsInPlay, bondsInPlay); compareRingAtomsConcerningNumNeighbors(atoms, nAts, mol); - ActivatePartitions(nAts, order, count.get(), activeset, next.get(), - changed.get()); - RefinePartitions(mol, atoms, sftor, true, order, count.get(), activeset, - next.get(), changed.get(), touched.get()); + ActivatePartitions(nAts, order, count, activeset, next, changed); + RefinePartitions(mol, atoms, sftor, true, order, count, activeset, next, + changed, touched); #ifdef VERBOSE_CANON std::cerr << "2b--------" << std::endl; for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) { @@ -350,8 +345,8 @@ void rankWithFunctor(T &ftor, bool breakTies, int *order, bool useSpecial, #endif } if (breakTies) { - BreakTies(mol, atoms, ftor, true, order, count.get(), activeset, next.get(), - changed.get(), touched.get()); + BreakTies(mol, atoms, ftor, true, order, count, activeset, next, changed, + touched); #ifdef VERBOSE_CANON std::cerr << "3--------" << std::endl; for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) { @@ -782,8 +777,8 @@ void rankMolAtoms(const ROMol &mol, std::vector &res, ftor.df_useNonStereoRanks = useNonStereoRanks; ftor.df_useChiralPresence = includeChiralPresence; - auto order = std::make_unique(mol.getNumAtoms()); - detail::rankWithFunctor(ftor, breakTies, order.get(), true, includeChirality, + std::vector order(mol.getNumAtoms()); + detail::rankWithFunctor(ftor, breakTies, order, true, includeChirality, includeRingStereo); for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) { @@ -832,8 +827,8 @@ void rankFragmentAtoms(const ROMol &mol, std::vector &res, ftor.df_useChiralityRings = includeChirality; ftor.df_useChiralPresence = includeChiralPresence; - auto order = std::make_unique(mol.getNumAtoms()); - detail::rankWithFunctor(ftor, breakTies, order.get(), true, includeChirality, + std::vector order(mol.getNumAtoms()); + detail::rankWithFunctor(ftor, breakTies, order, true, includeChirality, includeRingStereo, &atomsInPlay, &bondsInPlay); for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) { @@ -861,8 +856,8 @@ void chiralRankMolAtoms(const ROMol &mol, std::vector &res) { detail::initChiralCanonAtoms(mol, atoms); ChiralAtomCompareFunctor ftor(&atoms.front(), mol); - auto order = std::make_unique(mol.getNumAtoms()); - detail::rankWithFunctor(ftor, false, order.get()); + std::vector order(mol.getNumAtoms()); + detail::rankWithFunctor(ftor, false, order); for (unsigned int i = 0; i < mol.getNumAtoms(); ++i) { res[order[i]] = atoms[order[i]].index; diff --git a/Code/GraphMol/new_canon.h b/Code/GraphMol/new_canon.h index 4ec52f2e1..6e8ddbc34 100644 --- a/Code/GraphMol/new_canon.h +++ b/Code/GraphMol/new_canon.h @@ -694,12 +694,13 @@ class RDKIT_GRAPHMOL_EXPORT ChiralAtomCompareFunctor { template void RefinePartitions(const ROMol &mol, canon_atom *atoms, CompareFunc compar, - int mode, int *order, int *count, int &activeset, - int *next, int *changed, char *touchedPartitions) { + int mode, std::vector &order, + std::vector &count, int &activeset, + std::vector &next, std::vector &changed, + std::vector &touchedPartitions) { unsigned int nAtoms = mol.getNumAtoms(); int partition; int symclass = 0; - int *start; int offset; int index; int len; @@ -725,10 +726,10 @@ void RefinePartitions(const ROMol &mol, canon_atom *atoms, CompareFunc compar, len = count[partition]; offset = atoms[partition].index; - start = order + offset; + auto start = std::span(&order[offset], len); // std::cerr<<"\n\n**************************************************************"< void BreakTies(const ROMol &mol, canon_atom *atoms, CompareFunc compar, - int mode, int *order, int *count, int &activeset, int *next, - int *changed, char *touchedPartitions) { + int mode, std::vector &order, std::vector &count, + int &activeset, std::vector &next, + std::vector &changed, + std::vector &touchedPartitions) { unsigned int nAtoms = mol.getNumAtoms(); int partition; int offset; @@ -842,12 +845,13 @@ void BreakTies(const ROMol &mol, canon_atom *atoms, CompareFunc compar, } // end of BreakTies() RDKIT_GRAPHMOL_EXPORT void CreateSinglePartition(unsigned int nAtoms, - int *order, int *count, + std::vector &order, + std::vector &count, canon_atom *atoms); -RDKIT_GRAPHMOL_EXPORT void ActivatePartitions(unsigned int nAtoms, int *order, - int *count, int &activeset, - int *next, int *changed); +RDKIT_GRAPHMOL_EXPORT void ActivatePartitions( + unsigned int nAtoms, std::vector &order, std::vector &count, + int &activeset, std::vector &next, std::vector &changed); //! Note that atom maps on dummy atoms will always be used RDKIT_GRAPHMOL_EXPORT void rankMolAtoms( @@ -899,7 +903,7 @@ void initFragmentCanonAtoms(const ROMol &mol, const boost::dynamic_bitset<> &bondsInPlay, bool needsInit); template -void rankWithFunctor(T &ftor, bool breakTies, int *order, +void rankWithFunctor(T &ftor, bool breakTies, std::vector &order, bool useSpecial = false, bool useChirality = false, bool includeRingStereo = true, const boost::dynamic_bitset<> *atomsInPlay = nullptr, @@ -908,4 +912,4 @@ void rankWithFunctor(T &ftor, bool breakTies, int *order, } // namespace detail } // namespace Canon -} // namespace RDKit \ No newline at end of file +} // namespace RDKit diff --git a/Code/RDGeneral/hanoiSort.h b/Code/RDGeneral/hanoiSort.h index 69a63d1f8..44fad460b 100644 --- a/Code/RDGeneral/hanoiSort.h +++ b/Code/RDGeneral/hanoiSort.h @@ -1,5 +1,5 @@ // -// Copyright (C) 2014 Greg Landrum +// Copyright (C) 2014-2025 Greg Landrum and other RDKit contributors // Adapted from pseudo-code from Roger Sayle // // @@ All Rights Reserved @@ @@ -10,18 +10,21 @@ // #include -#ifndef HANOISORT_H_ -#define HANOISORT_H_ +#ifndef HANOISORT_H +#define HANOISORT_H #include #include #include +#include +#include #if defined(_MSC_VER) #pragma warning(push, 1) #pragma warning(disable : 4800) #endif namespace RDKit { +namespace detail { template bool hanoi(int *base, int nel, int *temp, int *count, int *changed, CompareFunc compar) { @@ -145,17 +148,25 @@ bool hanoi(int *base, int nel, int *temp, int *count, int *changed, } } } - +} // namespace detail template +[[deprecated("Use the overload that takes std::span and std::vector instead")]] void hanoisort(int *base, int nel, int *count, int *changed, CompareFunc compar) { assert(base); - int *temp = (int *)malloc(nel * sizeof(int)); - assert(temp); - if (hanoi(base, nel, temp, count, changed, compar)) { - memmove(base, temp, nel * sizeof(int)); + std::vector tempVec(nel); + if (detail::hanoi(base, nel, tempVec.data(), count, changed, compar)) { + memmove(base, tempVec.data(), nel * sizeof(int)); + } +} +template +void hanoisort(std::span &base, std::vector &count, + std::vector &changed, CompareFunc compar) { + std::vector tempVec(base.size()); + if (detail::hanoi(base.data(), base.size(), tempVec.data(), count.data(), + changed.data(), compar)) { + std::copy(tempVec.begin(), tempVec.end(), base.begin()); } - free(temp); } } // namespace RDKit diff --git a/ReleaseNotes.md b/ReleaseNotes.md index eaab7f875..479e1e096 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -43,6 +43,8 @@ GitHub) ## Code removed in this release: ## Deprecated code (to be removed in a future release): +- The version of hanoiSort() that takes raw pointers has been deprecated. Please use the version that takes std::span and std::vector. + # Release_2025.09.1