RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.25k stars 1.25k forks source link

Work on RigidBodyTree #3908

Closed amcastro-tri closed 7 years ago

amcastro-tri commented 7 years ago

A checklist to keep track of the planned work on RBT this quarter. I'll keep updating as I move along.

tkoolen commented 7 years ago

methods to query state dependent results

I mentioned this before, but I would really recommend moving these to KinematicsCache, so that the cache elements inside of it no longer need to be exposed, and so that it can be merged back with KinematicsResults again.

tkoolen commented 7 years ago

See also https://github.com/RobotLocomotion/drake/issues/1476.

amcastro-tri commented 7 years ago

Thank you for that list @tkoolen! Regarding your proposal, I was actually thinking more like the KinematicsCache would be this object the user needs to drag around representing the state of the world but the user actually doesn't really know what it is or how to interact with. The user would rather interact with it performing queries on it through RigidBodyTree. For instance, something like const Isometry3<T>& RigidBody<T>::get_pose(const KinematicsCache<T>& cache) const.

In this regard I was thinking to a similar philosophy in System2 where you have the pair System(const)/Context(non-const). In the dynamics world we'd then have the pair RigidBodyTree(const)/KinematicsCache(non-const). Also KinematicsCache will migrate to a System2 context and cache entries representation.

Another thing I like about this is that both API's for Systme2 and dynamics would have similar semantics, homogenizing API's across our code base. What do you think?

sherm1 commented 7 years ago

And I would like us to get to the point soon where we are literally using the System 2 Context in the RBTree API (in place of KinematicsCache), so that we don't have two separate caching schemes and a bunch of unnecessary pack/unpack/copy operations.

tkoolen commented 7 years ago

the KinematicsCache would be this object the user needs to drag around representing the state of the world but the user actually doesn't really know what it is or how to interact with

That's the current status quo, and the bad thing about it is that the internals of the cache need to be exposed in this case (this being the main reason for the creation of KinematicsResults).

SeanCurtis-TRI commented 7 years ago

@sherm1 I'm a bit wary of this. We'd talked about the need to be able to run RBT independent of the system 2 architecture. Would passing a System 2 Context instance be consistent with that?

david-german-tri commented 7 years ago

I haven't thought deeply about this, but I have the same knee-jerk reaction as @SeanCurtis-TRI. Surely RBTree can just consume some (set of) mutable pointer(s) to AwesomeFutureKinematicsData, which may or may not point into a System2 cache entry?

sherm1 commented 7 years ago

Most of the real value of the Context's caching utility won't be apparent until we are using it in RBTree, where really expensive things get calculated and reused frequently. Also, when an RBTree is used as a component in a System, most of the state variables and computational cost will come from the RBTree. I think it will prove much easier to do this directly in the RBTree API than to implement and maintain an interface layer to keep RBTree Context-free (to coin a phrase :). RBTree already has a similar dependency on KinematicsCache; we should just replace that with Context which is a more general version.