Issue #521: core: replace MultiMap with standard library container

Replace std::multimap with std::map of vectors. Fixes randomly failing composition/basic test due to an incorrect ordering of fragments
epam · Dec 7, 2022 · 71e469b · 71e469b
1 parent 1d1e43c
commit 71e469b
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 16 deletions.
diff --git a/api/tests/integration/ref/composition/basic.py.out b/api/tests/integration/ref/composition/basic.py.out
@@ -367,15 +367,15 @@ molecule 01: C1(C2CCCC2)=C(C2CCCC2)C=CC=C1 |RG:_R1={C1CCCC1},{C1CCCC1}|
 		fragment 1: C1CCCC1
 		fragment 2: C1CCCC1
 ===================================================
-molecule 02: C1(C2CCCCC2)=C(C2CCCC2)C=CC=C1 |RG:_R1={C1CCCCC1},{C1CCCC1}|
+molecule 02: C1(C2CCCCC2)=C(C2CCCC2)C=CC=C1 |RG:_R1={C1CCCC1},{C1CCCCC1}|
 	rgroup 1
-		fragment 1: C1CCCCC1
-		fragment 2: C1CCCC1
+		fragment 1: C1CCCC1
+		fragment 2: C1CCCCC1
 ===================================================
-molecule 03: C1(C2=CC=CC=C2)=C(C2CCCC2)C=CC=C1 |RG:_R1={C1=CC=CC=C1},{C1CCCC1}|
+molecule 03: C1(C2=CC=CC=C2)=C(C2CCCC2)C=CC=C1 |RG:_R1={C1CCCC1},{C1=CC=CC=C1}|
 	rgroup 1
-		fragment 1: C1=CC=CC=C1
-		fragment 2: C1CCCC1
+		fragment 1: C1CCCC1
+		fragment 2: C1=CC=CC=C1
 ===================================================
 molecule 04: C1(C2CCCC2)=C(C2CCCCC2)C=CC=C1 |RG:_R1={C1CCCC1},{C1CCCCC1}|
 	rgroup 1
@@ -387,10 +387,10 @@ molecule 05: C1(C2CCCCC2)=C(C2CCCCC2)C=CC=C1 |RG:_R1={C1CCCCC1},{C1CCCCC1}|
 		fragment 1: C1CCCCC1
 		fragment 2: C1CCCCC1
 ===================================================
-molecule 06: C1(C2=CC=CC=C2)=C(C2CCCCC2)C=CC=C1 |RG:_R1={C1=CC=CC=C1},{C1CCCCC1}|
+molecule 06: C1(C2=CC=CC=C2)=C(C2CCCCC2)C=CC=C1 |RG:_R1={C1CCCCC1},{C1=CC=CC=C1}|
 	rgroup 1
-		fragment 1: C1=CC=CC=C1
-		fragment 2: C1CCCCC1
+		fragment 1: C1CCCCC1
+		fragment 2: C1=CC=CC=C1
 ===================================================
 molecule 07: C1(C2CCCC2)=C(C2=CC=CC=C2)C=CC=C1 |RG:_R1={C1CCCC1},{C1=CC=CC=C1}|
 	rgroup 1

diff --git a/core/indigo-core/molecule/src/molecule_rgroups_composition.cpp b/core/indigo-core/molecule/src/molecule_rgroups_composition.cpp
@@ -132,28 +132,35 @@ MoleculeIter::SourceRGroups::SourceRGroups(const MoleculeIter& m)
 {
     Array<int> fs;
     m._at.dump(fs);
-    std::multimap<int, int> rgroup2fragment;
+    std::map<int, std::vector<int>> rgroup2fragment;
     std::map<Fragment, int> fragment2count;
     for (auto i = 0; i < fs.size(); i++)
     {
         auto x = m._parent._fragment_coordinates(i, fs[i]);
-        rgroup2fragment.emplace(x.rgroup, x.fragment);
+        const auto it = rgroup2fragment.find(x.rgroup);
+        if (it != rgroup2fragment.end())
+        {
+            it->second.push_back(x.fragment);
+        }
+        else
+        {
+            rgroup2fragment.emplace(x.rgroup, std::vector<int>{x.fragment});
+        }
         fragment2count.emplace(x, 1);
     }
 
-    for (auto it = rgroup2fragment.begin(); it != rgroup2fragment.end(); it = rgroup2fragment.upper_bound(it->first))
+    for (auto it = rgroup2fragment.begin(); it != rgroup2fragment.end(); it++)
     {
         RGroup& rgroup = _rgroups.push();
         RGroup& source = m._parent._rgroups.getRGroup(it->first);
 
-        const auto it_end = rgroup2fragment.upper_bound(it->first);
-        for (auto it_fs_r = rgroup2fragment.lower_bound(it->first); it_fs_r != it_end; it_fs_r++)
+        for (auto it_fs_r = it->second.begin(); it_fs_r != it->second.end(); it_fs_r++)
         {
-            for (auto k = 0; k < fragment2count.at({it->first, it_fs_r->second}); k++)
+            for (auto k = 0; k < fragment2count.at({it->first, *it_fs_r}); k++)
             {
                 int fr_idx = rgroup.fragments.add(new Molecule());
                 BaseMolecule* fragment = rgroup.fragments.at(fr_idx);
-                fragment->clone(*source.fragments[it_fs_r->second], nullptr, nullptr);
+                fragment->clone(*source.fragments[*it_fs_r], nullptr, nullptr);
                 fragment->removeAttachmentPoints();
             }
         }