Changeset c27778

Timestamp:

Jul 3, 2010, 2:02:26 PM (14 years ago)

Author:

Frederik Heber <heber@…>

Branches:

Action_Thermostats, Add_AtomRandomPerturbation, Add_FitFragmentPartialChargesAction, Add_RotateAroundBondAction, Add_SelectAtomByNameAction, Added_ParseSaveFragmentResults, AddingActions_SaveParseParticleParameters, Adding_Graph_to_ChangeBondActions, Adding_MD_integration_tests, Adding_ParticleName_to_Atom, Adding_StructOpt_integration_tests, AtomFragments, Automaking_mpqc_open, AutomationFragmentation_failures, Candidate_v1.5.4, Candidate_v1.6.0, Candidate_v1.6.1, ChangeBugEmailaddress, ChangingTestPorts, ChemicalSpaceEvaluator, CombiningParticlePotentialParsing, Combining_Subpackages, Debian_Package_split, Debian_package_split_molecuildergui_only, Disabling_MemDebug, Docu_Python_wait, EmpiricalPotential_contain_HomologyGraph, EmpiricalPotential_contain_HomologyGraph_documentation, Enable_parallel_make_install, Enhance_userguide, Enhanced_StructuralOptimization, Enhanced_StructuralOptimization_continued, Example_ManyWaysToTranslateAtom, Exclude_Hydrogens_annealWithBondGraph, FitPartialCharges_GlobalError, Fix_BoundInBox_CenterInBox_MoleculeActions, Fix_ChargeSampling_PBC, Fix_ChronosMutex, Fix_FitPartialCharges, Fix_FitPotential_needs_atomicnumbers, Fix_ForceAnnealing, Fix_IndependentFragmentGrids, Fix_ParseParticles, Fix_ParseParticles_split_forward_backward_Actions, Fix_PopActions, Fix_QtFragmentList_sorted_selection, Fix_Restrictedkeyset_FragmentMolecule, Fix_StatusMsg, Fix_StepWorldTime_single_argument, Fix_Verbose_Codepatterns, Fix_fitting_potentials, Fixes, ForceAnnealing_goodresults, ForceAnnealing_oldresults, ForceAnnealing_tocheck, ForceAnnealing_with_BondGraph, ForceAnnealing_with_BondGraph_continued, ForceAnnealing_with_BondGraph_continued_betteresults, ForceAnnealing_with_BondGraph_contraction-expansion, FragmentAction_writes_AtomFragments, FragmentMolecule_checks_bonddegrees, GeometryObjects, Gui_Fixes, Gui_displays_atomic_force_velocity, ImplicitCharges, IndependentFragmentGrids, IndependentFragmentGrids_IndividualZeroInstances, IndependentFragmentGrids_IntegrationTest, IndependentFragmentGrids_Sole_NN_Calculation, JobMarket_RobustOnKillsSegFaults, JobMarket_StableWorkerPool, JobMarket_unresolvable_hostname_fix, MoreRobust_FragmentAutomation, ODR_violation_mpqc_open, PartialCharges_OrthogonalSummation, PdbParser_setsAtomName, PythonUI_with_named_parameters, QtGui_reactivate_TimeChanged_changes, Recreated_GuiChecks, Rewrite_FitPartialCharges, RotateToPrincipalAxisSystem_UndoRedo, SaturateAtoms_findBestMatching, SaturateAtoms_singleDegree, StoppableMakroAction, Subpackage_CodePatterns, Subpackage_JobMarket, Subpackage_LinearAlgebra, Subpackage_levmar, Subpackage_mpqc_open, Subpackage_vmg, Switchable_LogView, ThirdParty_MPQC_rebuilt_buildsystem, TrajectoryDependenant_MaxOrder, TremoloParser_IncreasedPrecision, TremoloParser_MultipleTimesteps, TremoloParser_setsAtomName, Ubuntu_1604_changes, stable

Children:

2a374e

Parents:

b9c847

git-author:

Frederik Heber <heber@…> (07/03/10 13:24:25)

git-committer:

Frederik Heber <heber@…> (07/03/10 14:02:26)

Message:

MEMFIXES: ListOfLocalAtoms in molecule::FragmentMolecule() was not free'd correctly.

• FragmentationDepthFirstSearchAction::performCall() - uses now just ListOfAtoms, free'd and set to NULL in while loop
• changed signature: FillBondStructureFromReference(), FillListOfLocalAtoms() do not get FragmentCounter and ListOfLocalAtoms is now atom (i.e. directly the local list of this fragment not the global one)
• MoleculeLeafClass::FillListOfLocalAtoms() - simplified a lot due to the above
• no more removal of global list if this is the first fragment in MoleculeLeafClass::FillBondStructureFromReference(), MoleculeLeafClass::AssignKeySetsToFragment()
• molecule::FragmentMolecule() - for one we just use ListOfAtoms and around AssignKeySetstoFragments() call we properly allocate, set each component to NULL and free it afterwards.

NOTE: All of these lists and maps are hard to understand and make the code very confusing. It's really high time for refactoring.

Signed-off-by: Frederik Heber <heber@…>

Location:

src

Files:

• Actions/FragmentationAction/DepthFirstSearchAction.cpp (modified) (2 diffs)
• molecule.hpp (modified) (1 diff)
• molecule_fragmentation.cpp (modified) (4 diffs)
• molecule_graph.cpp (modified) (3 diffs)
• moleculelist.cpp (modified) (7 diffs)

src/Actions/FragmentationAction/DepthFirstSearchAction.cpp

@@ -49 +49 @@
     MoleculeLeafClass *Subgraphs = NULL;      // list of subgraphs from DFS analysis
     int *MinimumRingSize = new int[mol->getAtomCount()];
-    atom ***ListOfLocalAtoms = NULL;
+    atom **ListOfAtoms = NULL;
     class StackClass<bond *> *BackEdgeStack = NULL;
     class StackClass<bond *> *LocalBackEdgeStack = NULL;
@@ -58 +58 @@
       while (Subgraphs->next != NULL) {
         Subgraphs = Subgraphs->next;
-        Subgraphs->FillBondStructureFromReference(mol, FragmentCounter, ListOfLocalAtoms, false);  // we want to keep the created ListOfLocalAtoms
+        ListOfAtoms = NULL;
+        Subgraphs->FillBondStructureFromReference(mol, ListOfAtoms, false);  // we want to keep the created ListOfLocalAtoms
         LocalBackEdgeStack = new StackClass<bond *> (Subgraphs->Leaf->BondCount);
-        Subgraphs->Leaf->PickLocalBackEdges(ListOfLocalAtoms[FragmentCounter], BackEdgeStack, LocalBackEdgeStack);
+        Subgraphs->Leaf->PickLocalBackEdges(ListOfAtoms, BackEdgeStack, LocalBackEdgeStack);
         Subgraphs->Leaf->CyclicStructureAnalysis(LocalBackEdgeStack, MinimumRingSize);
         delete(LocalBackEdgeStack);
         delete(Subgraphs->previous);
+        delete[](ListOfAtoms);  // and here we remove it
         FragmentCounter++;
       }
       delete(Subgraphs);
-      for (int i=0;i<FragmentCounter;i++)
-        delete[](ListOfLocalAtoms[i]);
-      delete[](ListOfLocalAtoms);
     }
     delete(BackEdgeStack);

src/molecule.hpp

@@ -426 +426 @@
 
   bool AddLeaf(molecule *ptr, MoleculeLeafClass *Previous);
-  bool FillBondStructureFromReference(const molecule * const reference, int &FragmentCounter, atom ***&ListOfLocalAtoms, bool FreeList = false);
+  bool FillBondStructureFromReference(const molecule * const reference, atom **&ListOfLocalAtoms, bool FreeList = false);
   bool FillRootStackForSubgraphs(KeyStack *&RootStack, bool *AtomMask, int &FragmentCounter);
   bool AssignKeySetsToFragment(molecule *reference, Graph *KeySetList, atom ***&ListOfLocalAtoms, Graph **&FragmentList, int &FragmentCounter, bool FreeList = false);
-  bool FillListOfLocalAtoms(atom ***&ListOfLocalAtoms, const int FragmentCounter, const int GlobalAtomCount, bool &FreeList);
+  bool FillListOfLocalAtoms(atom **&ListOfLocalAtoms, const int GlobalAtomCount, bool &FreeList);
   void TranslateIndicesToGlobalIDs(Graph **FragmentList, int &FragmentCounter, int &TotalNumberOfKeySets, Graph &TotalGraph);
   int Count() const;

src/molecule_fragmentation.cpp

@@ -629 +629 @@
     MinimumRingSize[i] = getAtomCount();
   MolecularWalker = Subgraphs;
+  const int LeafCount = Subgraphs->next->Count();
   FragmentCounter = 0;
   while (MolecularWalker->next != NULL) {
     MolecularWalker = MolecularWalker->next;
     // fill the bond structure of the individually stored subgraphs
-    MolecularWalker->FillBondStructureFromReference(this, FragmentCounter, ListOfLocalAtoms, false);  // we want to keep the created ListOfLocalAtoms
+    ListOfAtoms = NULL;
+    MolecularWalker->FillBondStructureFromReference(this, ListOfAtoms, false);  // we want to keep the created ListOfLocalAtoms
     DoLog(0) && (Log() << Verbose(0) << "Analysing the cycles of subgraph " << MolecularWalker->Leaf << " with nr. " << FragmentCounter << "." << endl);
     LocalBackEdgeStack = new StackClass<bond *> (MolecularWalker->Leaf->BondCount);
@@ -644 +646 @@
 //        Log() << Verbose(0) << "\t" << ListOfLocalAtoms[FragmentCounter][i]->Name;
     DoLog(0) && (Log() << Verbose(0) << "Gathering local back edges for subgraph " << MolecularWalker->Leaf << " with nr. " << FragmentCounter << "." << endl);
-    MolecularWalker->Leaf->PickLocalBackEdges(ListOfLocalAtoms[FragmentCounter++], BackEdgeStack, LocalBackEdgeStack);
+    MolecularWalker->Leaf->PickLocalBackEdges(ListOfAtoms, BackEdgeStack, LocalBackEdgeStack);
     DoLog(0) && (Log() << Verbose(0) << "Analysing the cycles of subgraph " << MolecularWalker->Leaf << " with nr. " << FragmentCounter << "." << endl);
     MolecularWalker->Leaf->CyclicStructureAnalysis(LocalBackEdgeStack, MinimumRingSize);
     DoLog(0) && (Log() << Verbose(0) << "Done with Analysing the cycles of subgraph " << MolecularWalker->Leaf << " with nr. " << FragmentCounter << "." << endl);
     delete(LocalBackEdgeStack);
+    delete(ListOfAtoms);
+    FragmentCounter++;
   }
   delete(BackEdgeStack);
@@ -660 +664 @@
   // =================================== Begin of FRAGMENTATION ===============================
   // ===== 6a. assign each keyset to its respective subgraph =====
-  Subgraphs->next->AssignKeySetsToFragment(this, ParsedFragmentList, ListOfLocalAtoms, FragmentList, (FragmentCounter = 0), true);
+  ListOfLocalAtoms = new atom **[LeafCount];
+  for (int i=0;i<LeafCount;i++)
+    ListOfLocalAtoms[i] = NULL;
+  FragmentCounter = 0;
+  Subgraphs->next->AssignKeySetsToFragment(this, ParsedFragmentList, ListOfLocalAtoms, FragmentList, FragmentCounter, true);
+  delete[](ListOfLocalAtoms);
 
   // ===== 6b. prepare and go into the adaptive (Order<0), single-step (Order==0) or incremental (Order>0) cycle
@@ -695 +704 @@
   delete(ParsedFragmentList);
   delete[](MinimumRingSize);
-
 
   // ==================================== End of FRAGMENTATION ============================================

src/molecule_graph.cpp

@@ -664 +664 @@
   BFS.ColorList = new enum Shading[AtomCount];
   BFS.BFSStack = new StackClass<atom *> (AtomCount);
+  BFS.TouchedStack = new StackClass<atom *> (AtomCount);
 
   for (int i = AtomCount; i--;) {
     BFS.ShortestPathList[i] = -1;
     BFS.PredecessorList[i] = 0;
+    BFS.ColorList[i] = white;
   }
 };
@@ -681 +683 @@
   delete[](BFS.ColorList);
   delete (BFS.BFSStack);
+  delete (BFS.TouchedStack);
   BFS.AtomCount = 0;
 };
@@ -825 +828 @@
       MinRingSize = RingSize;
   } else {
-    DoLog(1) && (Log() << Verbose(1) << "No ring containing " << *BFS.Root << " with length equal to or smaller than " << MinimumRingSize[Walker->GetTrueFather()->nr] << " found." << endl);
+    DoLog(1) && (Log() << Verbose(1) << "No ring containing " << *BFS.Root << " with length equal to or smaller than " << MinimumRingSize[BFS.Root->GetTrueFather()->nr] << " found." << endl);
   }
 };

src/moleculelist.cpp

@@ -1017 +1017 @@
  * \param *out output stream for debugging
  * \param *reference reference molecule with the bond structure to be copied
- * \param &FragmentCounter Counter needed to address \a **ListOfLocalAtoms
- * \param ***ListOfLocalAtoms Lookup table for each subgraph and index of each atom in \a *reference, may be NULL on start, then it is filled
+ * \param **&ListOfLocalAtoms Lookup table for this subgraph and index of each atom in \a *reference, may be NULL on start, then it is filled
  * \param FreeList true - ***ListOfLocalAtoms is free'd before return, false - it is not
  * \return true - success, false - faoilure
  */
-bool MoleculeLeafClass::FillBondStructureFromReference(const molecule * const reference, int &FragmentCounter, atom ***&ListOfLocalAtoms, bool FreeList)
+bool MoleculeLeafClass::FillBondStructureFromReference(const molecule * const reference, atom **&ListOfLocalAtoms, bool FreeList)
 {
   atom *OtherWalker = NULL;
@@ -1031 +1030 @@
   DoLog(1) && (Log() << Verbose(1) << "Begin of FillBondStructureFromReference." << endl);
   // fill ListOfLocalAtoms if NULL was given
-  if (!FillListOfLocalAtoms(ListOfLocalAtoms, FragmentCounter, reference->getAtomCount(), FreeList)) {
+  if (!FillListOfLocalAtoms(ListOfLocalAtoms, reference->getAtomCount(), FreeList)) {
     DoLog(1) && (Log() << Verbose(1) << "Filling of ListOfLocalAtoms failed." << endl);
     return false;
@@ -1048 +1047 @@
       AtomNo = Father->nr; // global id of the current walker
       for (BondList::const_iterator Runner = Father->ListOfBonds.begin(); Runner != Father->ListOfBonds.end(); (++Runner)) {
-        OtherWalker = ListOfLocalAtoms[FragmentCounter][(*Runner)->GetOtherAtom((*iter)->GetTrueFather())->nr]; // local copy of current bond partner of walker
+        OtherWalker = ListOfLocalAtoms[(*Runner)->GetOtherAtom((*iter)->GetTrueFather())->nr]; // local copy of current bond partner of walker
         if (OtherWalker != NULL) {
           if (OtherWalker->nr > (*iter)->nr)
             Leaf->AddBond((*iter), OtherWalker, (*Runner)->BondDegree);
         } else {
-          DoLog(1) && (Log() << Verbose(1) << "OtherWalker = ListOfLocalAtoms[" << FragmentCounter << "][" << (*Runner)->GetOtherAtom((*iter)->GetTrueFather())->nr << "] is NULL!" << endl);
+          DoLog(1) && (Log() << Verbose(1) << "OtherWalker = ListOfLocalAtoms[" << (*Runner)->GetOtherAtom((*iter)->GetTrueFather())->nr << "] is NULL!" << endl);
           status = false;
         }
@@ -1062 +1061 @@
   if ((FreeList) && (ListOfLocalAtoms != NULL)) {
     // free the index lookup list
-    delete[](ListOfLocalAtoms[FragmentCounter]);
-    if (FragmentCounter == 0) // first fragments frees the initial pointer to list
-      delete[](ListOfLocalAtoms);
+    delete[](ListOfLocalAtoms);
   }
   DoLog(1) && (Log() << Verbose(1) << "End of FillBondStructureFromReference." << endl);
@@ -1110 +1107 @@
 /** Fills a lookup list of father's Atom::nr -> atom for each subgraph.
  * \param *out output stream from debugging
- * \param ***ListOfLocalAtoms Lookup table for each subgraph and index of each atom in global molecule, may be NULL on start, then it is filled
- * \param FragmentCounter counts the fragments as we move along the list
+ * \param **&ListOfLocalAtoms Lookup table for each subgraph and index of each atom in global molecule, may be NULL on start, then it is filled
  * \param GlobalAtomCount number of atoms in the complete molecule
  * \param &FreeList true - ***ListOfLocalAtoms is free'd before return, false - it is not
- * \return true - success, false - failure
- */
-bool MoleculeLeafClass::FillListOfLocalAtoms(atom ***&ListOfLocalAtoms, const int FragmentCounter, const int GlobalAtomCount, bool &FreeList)
+ * \return true - success, false - failure (ListOfLocalAtoms != NULL)
+ */
+bool MoleculeLeafClass::FillListOfLocalAtoms(atom **&ListOfLocalAtoms, const int GlobalAtomCount, bool &FreeList)
 {
   bool status = true;
 
-  if (ListOfLocalAtoms == NULL) { // allocated initial pointer
-    // allocate and set each field to NULL
-    const int Counter = Count();
-    ASSERT(FragmentCounter < Counter, "FillListOfLocalAtoms: FragmenCounter greater than present fragments.");
-    ListOfLocalAtoms = new atom**[Counter];
-    if (ListOfLocalAtoms == NULL) {
-      FreeList = FreeList && false;
-      status = false;
-    }
-    for (int i=0;i<Counter;i++)
-      ListOfLocalAtoms[i] = NULL;
-  }
-
-  if ((ListOfLocalAtoms != NULL) && (ListOfLocalAtoms[FragmentCounter] == NULL)) { // allocate and fill list of this fragment/subgraph
-    status = status && Leaf->CreateFatherLookupTable(ListOfLocalAtoms[FragmentCounter], GlobalAtomCount);
+  if (ListOfLocalAtoms == NULL) { // allocate and fill list of this fragment/subgraph
+    status = status && Leaf->CreateFatherLookupTable(ListOfLocalAtoms, GlobalAtomCount);
     FreeList = FreeList && true;
-  }
+  } else
+    return false;
 
   return status;
@@ -1158 +1142 @@
   DoLog(1) && (Log() << Verbose(1) << "Begin of AssignKeySetsToFragment." << endl);
   // fill ListOfLocalAtoms if NULL was given
-  if (!FillListOfLocalAtoms(ListOfLocalAtoms, FragmentCounter, reference->getAtomCount(), FreeList)) {
+  if (!FillListOfLocalAtoms(ListOfLocalAtoms[FragmentCounter], reference->getAtomCount(), FreeList)) {
     DoLog(1) && (Log() << Verbose(1) << "Filling of ListOfLocalAtoms failed." << endl);
     return false;
@@ -1203 +1187 @@
     // free the index lookup list
     delete[](ListOfLocalAtoms[FragmentCounter]);
-    if (FragmentCounter == 0) // first fragments frees the initial pointer to list
-      delete[](ListOfLocalAtoms);
   }
   DoLog(1) && (Log() << Verbose(1) << "End of AssignKeySetsToFragment." << endl);