RobotLocomotion / drake

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

multibody: Remove usages of `MultibodyTree` outside of `plant/` and `tree/` #11085

Closed EricCousineau-TRI closed 7 months ago

EricCousineau-TRI commented 5 years ago

At 767ec8868:

$ cd multibody
$ find . -mindepth 1 -maxdepth 1 -type d ! -name tree ! -name plant | xargs grep -rnI MultibodyTree
./benchmarks/kuka_iiwa_robot/drake_kuka_iiwa_robot.h:34:      std::unique_ptr<internal::MultibodyTree<T>> tree, double time_step = 0.) {
./benchmarks/kuka_iiwa_robot/drake_kuka_iiwa_robot.h:68:/// This class is a MultibodyTree model for a 7-DOF Kuka iiwa robot arm.
./benchmarks/kuka_iiwa_robot/drake_kuka_iiwa_robot.h:164:    // between MultibodyPlant and MultibodyTree's ordering.
./benchmarks/kuka_iiwa_robot/drake_kuka_iiwa_robot.h:247:  const multibody::internal::MultibodyTree<T>& tree() const {
./benchmarks/kuka_iiwa_robot/drake_kuka_iiwa_robot.h:277:  // This model's MultibodyTree always has a built-in "world" body.
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.cc:35:    multibody::internal::MultibodyTree<T>* model) {
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.cc:50:unique_ptr<multibody::internal::MultibodyTree<T>>
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.cc:52:  // Create a mostly empty MultibodyTree (it has a built-in "world" body).
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.cc:54:  auto model = make_unique<multibody::internal::MultibodyTree<T>>();
./benchmarks/kuka_iiwa_robot/test/kuka_iiwa_robot_inverse_dynamics_test.cc:18:// Compare Drake's MultibodyTree forces and motion with MotionGenesis solution.
./benchmarks/kuka_iiwa_robot/test/kuka_iiwa_robot_kinematics_test.cc:16:// Compare Drake's MultibodyTree kinematics with MotionGenesis solution.
./benchmarks/kuka_iiwa_robot/test/kuka_iiwa_robot_kinematics_test.cc:52:// Verify methods for MultibodyTree::CalcPositionKinematicsCache(), comparing
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:23:  using MultibodyTree = multibody::internal::MultibodyTree<U>;
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:25:  /// Instantiate a builder to make a MultibodyTree model of the KUKA iiwa arm
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:29:  ///   If `true`, the model is finalized with MultibodyTree::Finalize().
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:46:  std::unique_ptr<MultibodyTree<T>> Build() const;
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:70:  //   The MultibodyTree to which the new joint will be added.
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:77:      MultibodyTree<T>* model);
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:137:  // Flag indicating if MultibodyTree::Finalize() will be called on the model.
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:143:/// This method makes a MultibodyTree model for a Kuka Iiwa arm as specified
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:147:/// MultibodyTree::world_body().
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:150:/// The new MultibodyTree model is finalized by MultibodyTree::Finalize() and
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:153:///   If `true`, the model is finalized with MultibodyTree::Finalize().
./benchmarks/kuka_iiwa_robot/make_kuka_iiwa_model.h:159:std::unique_ptr<multibody::internal::MultibodyTree<T>> MakeKukaIiwaModel(
./test_utilities/floating_body_plant.h:94:  const internal::MultibodyTree<T>& tree() const {

EDIT: Assigned per issue scrub mtg.

jwnimmer-tri commented 5 years ago

Maybe @mitiguy could work on this?

sherm1 commented 5 years ago

Q4 is fully booked but looks like a reasonable tech debt reduction for 2020.

hongkai-dai commented 4 years ago

in #13921 in order to implement global inverse kinematics, I have to use some functions in MultibodyTree. The reason is that I need to go through the whole tree, and parse the kinematics relationship between adjacent links. The functions I called include

EricCousineau-TRI commented 4 years ago

I don't think get_topology() or get_mobilizer() are at all necessary in the PR. See review comments: https://github.com/RobotLocomotion/drake/pull/13921#pullrequestreview-488032873

It should be easy to upgrade their usages?

jwnimmer-tri commented 2 years ago

Luckily, one year later the kuka_iiwa_robot benchmark is still the only abuser of the MultibodyTree API, so we aren't adding new abuses. It would still be nice to clear away the misleading example from our codebase, though.

jwnimmer-tri commented 1 year ago

A few new uses have cropped up in the past 18 months:

./geometry/optimization/cspace_free_internal.cc:43:  const multibody::internal::MultibodyTree<double>& tree =
./multibody/rational/rational_forward_kinematics_internal.cc:19:  const MultibodyTreeTopology& topology = GetInternalTree(plant).get_topology();
./multibody/rational/rational_forward_kinematics_internal.cc:72:  const MultibodyTree<double>& tree = GetInternalTree(plant);
./multibody/rational/rational_forward_kinematics_internal.cc:76:      // path[i] is the child of path[i+1] in MultibodyTreeTopology, they are
./multibody/rational/rational_forward_kinematics_internal.cc:80:      // path[i] is the parent of path[i+1] in MultibodyTreeTopology, they are
./multibody/rational/rational_forward_kinematics_internal.cc:98:  const MultibodyTree<double>& tree = GetInternalTree(plant);
./multibody/rational/rational_forward_kinematics_internal.h:25: * `MultibodyTreeTopology`.
./multibody/rational/rational_forward_kinematics.h:240:   from that in MultibodyTreeTopology. If we designate a certain body as the
./multibody/rational/rational_forward_kinematics.h:243:   MultibodyTreeTopology usually designates the world as the root body, but we
./multibody/rational/rational_forward_kinematics.h:246:   might be flipped from that defined in MultibodyTreeTopology.
./multibody/rational/rational_forward_kinematics.cc:36: * MultibodyTree's or RationalForwardKinematics's definition; this function
./multibody/rational/rational_forward_kinematics.cc:64:  const internal::MultibodyTree<double>& tree = GetInternalTree(plant_);
./multibody/rational/rational_forward_kinematics.cc:233:  const internal::MultibodyTree<double>& tree = GetInternalTree(plant_);
mitiguy commented 1 year ago

I'll look at the kuka_iiwa_robot abuses.

jwnimmer-tri commented 7 months ago

I'm not clear what the problem is here. In general, internal code inside Drake is allowed to call "internal" functions. Having code in Drake use MbT directly doesn't seem like a de-jure defect. If there are acute problems with using MbT in certain places, let's deal with those specifically when they come up.