[Yade-dev] Test needed from Clumps-users

2014-05-16 Thread Anton Gladky
Dear users/devs, who is working with clumps,

I have done a tiny modification in NewtonIntegrator [1], which
should fix numerical instability with clumps in a rare cases.



Subject: [PATCH] Do not use getTorqueUnsynced and forces.getForceUnsynced in
 clumps

It seems both of these commands are unsafe. It causes NaN velocities
in a very specific cases. From my point of view it is OK, because
addForceTorqueFromMembers does not modify data itself.
---
 pkg/dem/NewtonIntegrator.cpp | 6 --
 1 file changed, 6 deletions(-)

diff --git a/pkg/dem/NewtonIntegrator.cpp b/pkg/dem/NewtonIntegrator.cpp
index e961bf2..082add8 100644
--- a/pkg/dem/NewtonIntegrator.cpp
+++ b/pkg/dem/NewtonIntegrator.cpp
@@ -146,14 +146,8 @@ void NewtonIntegrator::action()
  // clumps forces
  if(b-isClump()) {
  b-shape-castClump().addForceTorqueFromMembers(state,scene,f,m);
- #ifdef YADE_OPENMP
- //it is safe here, since only one thread will read/write
- scene-forces.getTorqueUnsynced(id)+=m;
- scene-forces.getForceUnsynced(id)+=f;
- #else
  scene-forces.addTorque(id,m);
  scene-forces.addForce(id,f);
- #endif
  }
  //in most cases, the initial force on clumps will be zero and next
line is not changing f and m, but make sure we don't miss something
(e.g. user defined forces on clumps)
  f=scene-forces.getForce(id); m=scene-forces.getTorque(id);
-- 
1.9.3



Please, test you scripts with the newest version. Yadedaily should
get this fix tonight.

Comments are very welcome.

[1] 
https://github.com/yade/trunk/commit/5707bb55fb9a3605e10a82cdecbec8efcc7d906f

Best regards

Anton

___
Mailing list: https://launchpad.net/~yade-dev
Post to : yade-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~yade-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Yade-dev] Test needed from Clumps-users

2014-05-16 Thread Bruno Chareyre
Hi Anton,
Two problems I suspect:
1/ It needs speed benchmarking. This little change may have a huge
impact on parallel performance, because - if I don't miss something -
each addForce/addTorque trigger the synchronization of the whole
force/torque containers [1], which is not small thing as you can see.
If this is done for each clump it makes 3 nested loop O(N² x numThreads).
If it occures as I think, it should be visible in O.forces.syncCount.
Normally this value is equal to the number of timesteps.
2/ It is not thread-safe. addForce/addTorque are ok in interaction loop
because it is write-only. Here different threads will try to
write-synchronize-read at the same time.

Bruno


[1] https://github.com/yade/trunk/blob/master/core/ForceContainer.hpp#L128


On 16/05/14 11:48, Anton Gladky wrote:
 Dear users/devs, who is working with clumps,

 I have done a tiny modification in NewtonIntegrator [1], which
 should fix numerical instability with clumps in a rare cases.

 
 
 Subject: [PATCH] Do not use getTorqueUnsynced and forces.getForceUnsynced in
  clumps

 It seems both of these commands are unsafe. It causes NaN velocities
 in a very specific cases. From my point of view it is OK, because
 addForceTorqueFromMembers does not modify data itself.
 ---
  pkg/dem/NewtonIntegrator.cpp | 6 --
  1 file changed, 6 deletions(-)

 diff --git a/pkg/dem/NewtonIntegrator.cpp b/pkg/dem/NewtonIntegrator.cpp
 index e961bf2..082add8 100644
 --- a/pkg/dem/NewtonIntegrator.cpp
 +++ b/pkg/dem/NewtonIntegrator.cpp
 @@ -146,14 +146,8 @@ void NewtonIntegrator::action()
   // clumps forces
   if(b-isClump()) {
   b-shape-castClump().addForceTorqueFromMembers(state,scene,f,m);
 - #ifdef YADE_OPENMP
 - //it is safe here, since only one thread will read/write
 - scene-forces.getTorqueUnsynced(id)+=m;
 - scene-forces.getForceUnsynced(id)+=f;
 - #else
   scene-forces.addTorque(id,m);
   scene-forces.addForce(id,f);
 - #endif
   }
   //in most cases, the initial force on clumps will be zero and next
 line is not changing f and m, but make sure we don't miss something
 (e.g. user defined forces on clumps)
   f=scene-forces.getForce(id); m=scene-forces.getTorque(id);


-- 
___
Bruno Chareyre
Associate Professor
ENSE³ - Grenoble INP
Lab. 3SR
BP 53
38041 Grenoble cedex 9
Tél : +33 4 56 52 86 21
Fax : +33 4 76 82 70 43



___
Mailing list: https://launchpad.net/~yade-dev
Post to : yade-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~yade-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Yade-dev] Test needed from Clumps-users

2014-05-16 Thread Anton Gladky
Hi all,

sorry for the noise. This change is reverted.

Anton


2014-05-16 11:48 GMT+02:00 Anton Gladky gladky.an...@gmail.com:
 Dear users/devs, who is working with clumps,

 I have done a tiny modification in NewtonIntegrator [1], which
 should fix numerical instability with clumps in a rare cases.

 
 
 Subject: [PATCH] Do not use getTorqueUnsynced and forces.getForceUnsynced in
  clumps

 It seems both of these commands are unsafe. It causes NaN velocities
 in a very specific cases. From my point of view it is OK, because
 addForceTorqueFromMembers does not modify data itself.
 ---
  pkg/dem/NewtonIntegrator.cpp | 6 --
  1 file changed, 6 deletions(-)

 diff --git a/pkg/dem/NewtonIntegrator.cpp b/pkg/dem/NewtonIntegrator.cpp
 index e961bf2..082add8 100644
 --- a/pkg/dem/NewtonIntegrator.cpp
 +++ b/pkg/dem/NewtonIntegrator.cpp
 @@ -146,14 +146,8 @@ void NewtonIntegrator::action()
   // clumps forces
   if(b-isClump()) {
   b-shape-castClump().addForceTorqueFromMembers(state,scene,f,m);
 - #ifdef YADE_OPENMP
 - //it is safe here, since only one thread will read/write
 - scene-forces.getTorqueUnsynced(id)+=m;
 - scene-forces.getForceUnsynced(id)+=f;
 - #else
   scene-forces.addTorque(id,m);
   scene-forces.addForce(id,f);
 - #endif
   }
   //in most cases, the initial force on clumps will be zero and next
 line is not changing f and m, but make sure we don't miss something
 (e.g. user defined forces on clumps)
   f=scene-forces.getForce(id); m=scene-forces.getTorque(id);
 --
 1.9.3
 
 

 Please, test you scripts with the newest version. Yadedaily should
 get this fix tonight.

 Comments are very welcome.

 [1] 
 https://github.com/yade/trunk/commit/5707bb55fb9a3605e10a82cdecbec8efcc7d906f

 Best regards

 Anton

___
Mailing list: https://launchpad.net/~yade-dev
Post to : yade-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~yade-dev
More help   : https://help.launchpad.net/ListHelp