search

osrf / drcsim

Repository for the DRC project.
16 stars 6 forks source link

SetDamping / GetDamping is totally wrong in AtlasPlugin.cpp #488

Closed osrf-migration closed 9 years ago

osrf-migration commented 9 years ago

Original report (archived issue) by Siyuan Feng (Bitbucket: siyuanfeng).

On the default branch (I also hacked the one in prerelease 4.2.5), SetDamping / GetDamping pair is not implemented correctly. On the default branch, line https://github.com/osrf/drcsim/blob/69be941fc16c7958d13222206bd9b43738308d91/drcsim_gazebo_ros_plugins/src/AtlasPlugin.cpp#L1147 and the subsequent line set the damping to 1 regardless of the default value or what the user sets.

For the GetJointDamping method exposed by the API, https://github.com/osrf/drcsim/blob/69be941fc16c7958d13222206bd9b43738308d91/drcsim_gazebo_ros_plugins/src/AtlasPlugin.cpp#L1298 it returns the internal value as opposed to the actual values that the joint uses.

I also do not understand the reasoning behind line https://github.com/osrf/drcsim/blob/69be941fc16c7958d13222206bd9b43738308d91/drcsim_gazebo_ros_plugins/src/AtlasPlugin.cpp#L2673 You have already set the joint damping in the physics engine, why are you explicitly adding a damping torque again?

There are many SetDamping call scattered around in AtlasPlugin.cpp, I suggest someone do a detailed cleanup.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

Thanks for the report. I'll address these in order:

  1. AtlasPlugin.cpp:1147: this looks like a botched merge in b41b4ea0a41dec0cab70f3562546e9ee945ba7de that slipped through code review. We will fix it.

  2. AtlasPlugin.cpp:1298: I'll check with @hsu about what is supposed to happen here.

  3. AtlasPlugin.cpp:2673: I believe this is correct. The velocity feedback term consists of jointDampingCoef * (targetVelocity - actualVelocity). The actualVelocity term is indeed handled by the physics engine, so we are just adding the targetVelocity (ie. this->atlasCommand.velocity[i]) term here.

osrf-migration commented 9 years ago

Original comment by John Hsu (Bitbucket: hsu, GitHub: hsu).

according to the ROS API table the function bool AtlasPlugin::GetJointDamping(...) Atlas.cpp:1298 is supposed to return the joint's viscous damping coefficient.

Looks like the table could use some content and cosmetic updates.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

I just made a quick pull request #530 to fix the botched merge (1 in list above).

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

So, @hsu, are you saying that GetJointDamping is working correctly?

osrf-migration commented 9 years ago

Original comment by John Hsu (Bitbucket: hsu, GitHub: hsu).

Yes, GetJointDamping is supposed to return the amount of viscous damping in the physical model (not the controller). So I believe it's doing the right thing. @siyuanfeng, are you looking for the overall damping the joint sees from a combination of joint controller and the physical model?

osrf-migration commented 9 years ago

Original comment by Siyuan Feng (Bitbucket: siyuanfeng).

yes

osrf-migration commented 9 years ago

Original comment by John Hsu (Bitbucket: hsu, GitHub: hsu).

@siyuanfeng the combined "damping coefficient" is a fictitious value that is not quite physical. We are using implicit damping to cheat and achieve stability for larger controller d-terms. You can lower the max joint damping value to match the actual joint damping value of the joint, and the controller d-terms will be applied explicitly, which is a more physically realistic scenario.

osrf-migration commented 9 years ago

Original comment by Siyuan Feng (Bitbucket: siyuanfeng).

ok, here is what I wanted, and let's figure out how to get there.

In my model, I am assuming a perfect second order model, and I am doing pure torque control. So I need to somehow compensate out the additive "damping" you added in the physics on in my feedforward torques. Right now, I am assuming a simple viscous damping model, which I believe is what you guys are trying to get in the infinite computational budget case. And for this simple model, I need to know the effective damping coefficient. I am open to more complex damping model, but at the end of the day, I need some model with some numbers from you guys that accurately reflect what's going on in the simulator.

I also think it's desirable for me to turn off damping completely, which is not an option in the current setup, since it always get min to the default. I might be wrong on this. I did try hack AtlasPlugin so that there is not damping, by getting rid of the additive force, and SetDamping(0,0), the simulator didn't blow up. And I got what I wanted as well in that case.

thanks

osrf-migration commented 9 years ago

Original comment by John Hsu (Bitbucket: hsu, GitHub: hsu).

@siyuanfeng

To get zero viscous joint damping from the physics solution, I believe you can achieve that by:

This should remove any physics based viscous joint damping for the simulated model. Let us know if this does what you need. If not, we can instrument something different?

osrf-migration commented 9 years ago

Original comment by Siyuan Feng (Bitbucket: siyuanfeng).

Sounds good to me, as long as I can set it to zero, I am happy. thanks

osrf-migration commented 9 years ago

Original comment by John Hsu (Bitbucket: hsu, GitHub: hsu).

Found workaround. Please reopen if needed.