Changeset c27778 for src/moleculelist.cpp

Timestamp:

Jul 3, 2010, 2:02:26 PM (15 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@…>

File:

• src/moleculelist.cpp (modified) (7 diffs)

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);