Yade

Bug #1474956
Comment #2

Comment 2 for bug 1474956

Revision history for this message

Bruno Chareyre (bruno-chareyre) wrote on 2015-07-21:

>it should be downward compatible

Sure of that? It makes things more complex without adding any additional flexibility.
As far as inertia matters, there is no point in subdiscretizing this tiny sphere which is 1000 times smaller that the clump.
But if you really want to do it you can: discretization=10000.
It is not possible the other way around since discretization can't be less than 1 (as explained above).
What is the advantage of defining resolution wrt the smallest element of an object overall?

I agree that downward compatibility is good when possible, but we can't keep it at all prices. Sometimes we have to keep things simple to.

>it needs a lot of extra code modification

Without the above optional behavior the fix fits in 3 changed lines (and one change is in fact unrelated, see below).

Thanks for pointing to the relevant doc section.

Bruno

p.s. Please don't cc me or anybody else when sending email to bug/aswers/lists because it generates duplicates

diff --git a/core/Clump.cpp b/core/Clump.cpp
index deb10b5..869cc5a 100644
--- a/core/Clump.cpp
+++ b/core/Clump.cpp
@@ -144,17 +144,18 @@ void Clump::updateProperties(const shared_ptr<Body>& clumpBody, unsigned int dis
      */
        if(intersecting){
- //get boundaries and minimum radius of clump:
- Real rMin=1./0.; AlignedBox3r aabb;
+ //get boundaries of clump:
+ AlignedBox3r aabb;
                FOREACH(MemberMap::value_type& mm, clump->members){
                        const shared_ptr<Body> subBody = Body::byId(mm.first);
                        if (subBody->shape->getClassIndex() == Sph_Index){//clump member should be a sphere
                                const Sphere* sphere = YADE_CAST<Sphere*> (subBody->shape.get());
                                aabb.extend(subBody->state->pos + Vector3r::Constant(sphere->radius));
                                aabb.extend(subBody->state->pos - Vector3r::Constant(sphere->radius));
- rMin=min(rMin,sphere->radius);
                        }
                }
+ Real rMin=min(aabb.diagonal()[0],min(aabb.diagonal()[1],aabb.diagonal()[2]));
+
                //get volume and inertia tensor using regular cubic cell array inside bounding box of the clump:
                Real dx = rMin/discretization; //edge length of cell
                Real dv = pow(dx,3); //volume of cell
@@ -182,8 +183,7 @@ void Clump::updateProperties(const shared_ptr<Body>& clumpBody, unsigned int dis
                                }
                        }
                }
- }
- if(!intersecting){
+ } else {//not intersecting
                FOREACH(MemberMap::value_type& mm, clump->members){
                        // mm.first is Body::id_t, mm.second is Se3r of that body
                        const shared_ptr<Body> subBody=Body::byId(mm.first);

>it should be downward compatible

I agree that downward compatibility is good when possible, but we can't keep it at all prices. Sometimes we have to keep things simple to.

>it needs a lot of extra code modification

Without the above optional behavior the fix fits in 3 changed lines (and one change is in fact unrelated, see below).

Thanks for pointing to the relevant doc section.

Bruno

p.s. Please don't cc me or anybody else when sending email to bug/aswers/lists because it generates duplicates

diff --git a/core/Clump.cpp b/core/Clump.cpp
index deb10b5..869cc5a 100644
--- a/core/Clump.cpp
+++ b/core/Clump.cpp
@@ -144,17 +144,18 @@ void Clump::updateProperties(const shared_ptr<Body>& clumpBody, unsigned int dis
      */
        if(intersecting){       
-               //get boundaries and minimum radius of clump:
-               Real rMin=1./0.; AlignedBox3r aabb;
+               //get boundaries of clump:
+               AlignedBox3r aabb;
                FOREACH(MemberMap::value_type& mm, clump->members){
                        const shared_ptr<Body> subBody = Body::byId(mm.first);
                        if (subBody->shape->getClassIndex() == Sph_Index){//clump member should be a sphere
                                const Sphere* sphere = YADE_CAST<Sphere*> (subBody->shape.get());
                                aabb.extend(subBody->state->pos + Vector3r::Constant(sphere->radius));
                                aabb.extend(subBody->state->pos - Vector3r::Constant(sphere->radius));
-                               rMin=min(rMin,sphere->radius);
                        }
                }
+               Real rMin=min(aabb.diagonal()[0],min(aabb.diagonal()[1],aabb.diagonal()[2])); 
+               
                //get volume and inertia tensor using regular cubic cell array inside bounding box of the clump:
                Real dx = rMin/discretization;  //edge length of cell
                Real dv = pow(dx,3);            //volume of cell
@@ -182,8 +183,7 @@ void Clump::updateProperties(const shared_ptr<Body>& clumpBody, unsigned int dis
                                }
                        }
                }
-       }
-       if(!intersecting){
+       } else {//not intersecting
                FOREACH(MemberMap::value_type& mm, clump->members){
                        // mm.first is Body::id_t, mm.second is Se3r of that body
                        const shared_ptr<Body> subBody=Body::byId(mm.first);