yade-dev team mailing list archive

[noreply@xxxxxxxxxxxxx]

------------------------------------------------------------
revno: 2180
committer: Václav Šmilauer <eudoxos@xxxxxxxx>
branch nick: trunk
timestamp: Mon 2010-04-26 00:07:12 +0200
message:
  REALLY fix bad end iterator on InteractionContainer (since r2092!!). UPDATE your code, this makes simulations little wrong (some interaction might be processed twice in one step).
modified:
  core/InteractionContainer.cpp
  core/InteractionContainer.hpp


--
lp:yade
https://code.launchpad.net/~yade-dev/yade/trunk

Your team Yade developers is subscribed to branch lp:yade.
To unsubscribe from this branch go to https://code.launchpad.net/~yade-dev/yade/trunk/+edit-subscription

=== modified file 'core/InteractionContainer.cpp'
--- core/InteractionContainer.cpp	2010-04-24 20:22:40 +0000
+++ core/InteractionContainer.cpp	2010-04-25 22:07:12 +0000
@@ -12,31 +12,23 @@
 	//BOOST_SERIALIZATION_FACTORY_0(InteractionContainer);
 #endif
 
-bool InteractionContainer::insert(const shared_ptr<Interaction>& i)
-{
+bool InteractionContainer::insert(const shared_ptr<Interaction>& i){
 	boost::mutex::scoped_lock lock(drawloopmutex);
-
-	body_id_t id1 = i->getId1();
-	body_id_t id2 = i->getId2();
-
-	if (id1>id2)
-		swap(id1,id2);
-
-	if ( static_cast<unsigned int>(id1) >=vecmap.size())
-		vecmap.resize(id1+1);
-
-	if (vecmap[id1].insert(pair<body_id_t,unsigned int >(id2,currentSize)).second)
-	{
-		if (intrs.size() == currentSize)
-			intrs.resize(currentSize+1);
-
-		intrs[currentSize]=i;
-		currentSize++;
+	body_id_t id1=i->getId1(), id2=i->getId2();
+	if (id1>id2) swap(id1,id2);
+
+	if((size_t)id1>=vecmap.size()) vecmap.resize(id1+1); // resize linear map to accomodate id1
+
+	// inserted element maps id2->currSize; currSize will be incremented immediately
+	if(!vecmap[id1].insert(pair<body_id_t,size_t>(id2,currSize)).second) return false; // id1,id2 pair already present
+		
+	assert(intrs.size()==currSize);
+	intrs.resize(++currSize); // currSize updated
+	assert(intrs.size()==currSize);
+
+	intrs[currSize-1]=i; // assign last element
 	
-		return true;
-	}
-	else
-		return false;
+	return true;
 }
 
 
@@ -47,73 +39,52 @@
 }
 
 
-void InteractionContainer::clear()
-{
+void InteractionContainer::clear(){
 	boost::mutex::scoped_lock lock(drawloopmutex);
 
 	vecmap.clear();
 	intrs.clear();
 	pendingErase.clear();
-	currentSize=0;
+	currSize=0;
 }
 
 
-bool InteractionContainer::erase(body_id_t id1,body_id_t id2)
-{
+bool InteractionContainer::erase(body_id_t id1,body_id_t id2){
 	boost::mutex::scoped_lock lock(drawloopmutex);
-
-	if (id1>id2)
-		swap(id1,id2);
-
-	if ( static_cast<unsigned int>(id1) < vecmap.size())
-	{
-		map<body_id_t,unsigned int >::iterator mii;
-		mii = vecmap[id1].find(id2);
-		if ( mii != vecmap[id1].end() )
-		{
-			unsigned int iid = (*mii).second;
-			vecmap[id1].erase(mii);
-			currentSize--;
-			if (iid<currentSize) {
-				intrs[iid]=intrs[currentSize];
-				id1 = intrs[iid]->getId1();
-				id2 = intrs[iid]->getId2();
-				if (id1>id2) swap(id1,id2);
-				vecmap[id1][id2]=iid;
-			}
-			return true;
-		}
-		else
-			return false;
+	if (id1>id2) swap(id1,id2);
+	if((size_t)id1<=vecmap.size()) return false; // id1 out of bounds
+	map<body_id_t,size_t>::iterator mii;
+	mii=vecmap[id1].find(id2);
+	if(mii==vecmap[id1].end()) return false; // id2 not in interaction with id1
+	// interaction found; erase from vecmap and then from intrs as well
+	size_t iid=(*mii).second;
+	vecmap[id1].erase(mii);
+	// iid is not the last element; we have to move last one to its place
+	if (iid<currSize-1) {
+		intrs[iid]=intrs[currSize-1];
+		// adjust map, so that id1,id2 points to element at iid, which used to be last
+		id1=intrs[iid]->getId1();
+		id2=intrs[iid]->getId2();
+		if (id1>id2) swap(id1,id2);
+		vecmap[id1][id2]=iid;
 	}
-
-	return false;
-
+	assert(intrs.size()==currSize);
+	// in either case, last element can be removed now
+	intrs.resize(--currSize); // currSize updated
+	assert(intrs.size()==currSize);
+	return true;
 }
 
 
-const shared_ptr<Interaction>& InteractionContainer::find(body_id_t id1,body_id_t id2)
-{
-	if (id1>id2)
-		swap(id1,id2);
-
-	if (static_cast<unsigned int>(id1)<vecmap.size())
-	{
-		map<body_id_t,unsigned int >::iterator mii;
-		mii = vecmap[id1].find(id2);
-		if (mii!=vecmap[id1].end())
-			return intrs[(*mii).second];
-		else
-		{
-			empty = shared_ptr<Interaction>();
-			return empty;
-		}
-	}
-	else
-	{
-		empty = shared_ptr<Interaction>();
-		return empty;
-	}
+const shared_ptr<Interaction>& InteractionContainer::find(body_id_t id1,body_id_t id2){
+	if (id1>id2) swap(id1,id2);
+
+	if ((size_t)id1>=vecmap.size()) { empty=shared_ptr<Interaction>(); return empty; }
+
+	map<body_id_t,size_t>::iterator mii;
+	mii = vecmap[id1].find(id2);
+	if (mii!=vecmap[id1].end()) return intrs[(*mii).second];
+	else { empty=shared_ptr<Interaction>(); return empty; }
 }
 
 void InteractionContainer::requestErase(body_id_t id1, body_id_t id2, bool force){

=== modified file 'core/InteractionContainer.hpp'
--- core/InteractionContainer.hpp	2010-03-21 15:25:50 +0000
+++ core/InteractionContainer.hpp	2010-04-25 22:07:12 +0000
@@ -33,14 +33,17 @@
 class InteractionContainer: public Serializable{
 	private :
 		typedef vector<shared_ptr<Interaction> > ContainerT;
+		// linear array of container interactions
 		vector<shared_ptr<Interaction> > intrs;
-		vector<map<body_id_t, unsigned int  > > vecmap;
-		unsigned int currentSize;
+		// array where vecmap[id1] maps id2 to index in intrs (unsigned int)
+		vector<map<body_id_t,size_t> > vecmap;
+		// always in sync with intrs.size()
+		size_t currSize;
 		shared_ptr<Interaction> empty;
 		// used only during serialization/deserialization
 		vector<shared_ptr<Interaction> > interaction;
 	public :
-		InteractionContainer(): currentSize(0),serializeSorted(false),iterColliderLastRun(-1){
+		InteractionContainer(): currSize(0),serializeSorted(false),iterColliderLastRun(-1){
 			#ifdef YADE_OPENMP
 				threadsPendingErase.resize(omp_get_max_threads());
 			#endif
@@ -61,7 +64,7 @@
 		// index access
 		shared_ptr<Interaction>& operator[](size_t id){return intrs[id];}
 		const shared_ptr<Interaction>& operator[](size_t id) const { return intrs[id];}
-		size_t size(){ return currentSize; }
+		size_t size(){ return currSize; }
 		// simulation API
 
 		//! Erase all non-real (in term of Interaction::isReal()) interactions