Replace Drake's SDF Parser with OSRF's SDF Parser

[liangfok]

Problem Definition

Drake currently implements its own SDF parser. Having and maintaining its own parser makes it difficult for Drake to remain in sync with changes in the official SDF specification.

Action Items

Let's replace this custom parser with OSRF's SDF parser (aka sdformat or libsdformat). This can be done by adding libsdformat as a Drake external and then using it to instantiate a sdf::SDF object, which can then be used to instantiate various Drake simulation objects like RigidBodyTree, etc.

[liangfok]

4333 is relevant since its resolution involves some level of work maintaining / fixing the current SDF parser.

[jwnimmer-tri]

For the record: it's pretty obvious to me that this the correct thing to do. The question now is the urgency, and how it inter-relates to other concurrent tasks. I've assigned it to myself while I help sort out that answer.

/CC @nkoenig

[liangfok]

In #4597 I added a DepthSensor but did not port System 1.0's custom SDF parser (it's located in RigidBodySystem). I'm thinking about going ahead and adding OSRF's SDF parser as a Drake external and using it to instantiate a DepthSensor based on an SDF specification. In other words, this would incrementally replace Drake's current custom SDF parser with OSRF's.

[jwnimmer-tri]

@sherm1 In #4633 you say "Build an MBTree from urdf and sdf, with fully-documented mapping from files to internal frames". Do you know how much that touches or doesn't the current XML-level parsing goop? If it's a lot, would it make sense for you to take over steering this issue?

[sherm1]

Yes, I think it makes great sense to do this for MBTree and not attempt an RBTree retrofit. I'll reassign to dynamics but it would be great to work with @liangfok on this.

[amcastro-tri]

@sherm1, I think that makes the most sense. Looking into the parsers' source I just see a very basic example. @liangfok, do you know of more complex usages cases to learn how that library works? is its API documented/used anywhere? What about URDF files, is there an OSRF library for urdf files as well?

I think if we go down this route (which I support) we will definitely like to have an intermediate representation for multiple parsers. After my quick dive into the SDF parser in #4615 I realize many of the problems there could be solved in a sensitive way with an IR. I would just start with an IR for MBTree since these parsers will be MBTree specific anyway.

[clalancette]

Has anyone started work on this yet? If not, I'll take a crack at it. From the comments above, I see that we are thinking about switching to an IR, but I think I'd like to start more simply (not least of which because I am just learning Drake). My plan would be to do something roughly like:

1. Import sdformat as an external.
2. Use sdformat to replace the very low-level details of sdf_parser.cc . At this stage, we wouldn't enable any additional features, just replace the existing implementation with the sdformat library.
3. Start adding additional APIs that might be needed by @liangfok
4. Think about doing some kind of IR layer on top.

What do people think about this rough plan? Would that work, or should I do something different?

[liangfok]

@clalancette, your plan sounds excellent. I haven't started work on this yet; thanks for picking this up!

[amcastro-tri]

Thank you for offering @clalancette! we'd love to have this functionality in!

Use sdformat to replace the very low-level details of sdf_parser.cc . At this stage, we wouldn't enable any additional features, just replace the existing implementation with the sdformat library.

I think that approach would require a lot of throwaway code since we will be soon migrating to a different implementation of our multibody engine (see #4633). I think a much better approach would be to start with an IR. Just a simple one (bodies, links, masses, inertias). Probably the SDF parser already provides this IR and we don't need anything new here? probably the IR could just be the same sdf::SDF()? As long as we can query this IR for things like: links, masses, inertias in body frame (or any other frame as long as well defined/documented), joints, axes, dampings ,etc. we should be good. The key is to have this IR (which again could be sdf::SDF()) that would allow us to query for these items whether we want to build a RigidBodyTree, a MultibodyTree or any other physics engine object.

[liangfok]

:+1: for using OSRF's libsdformat to create an IR. This IR can initially be used to instantiate sensors, which Drake's sdf_parser.h currently does not do. Then, once MultibodyTree exists, the IR can be used to instantiate it. I agree with @amcastro-tri's comment that we should try to avoid updating Drake's current sdf_parser. One thing I'm unsure about is whether (1) libsdformat should provide the IR, (2) existing classes within Drake should be used for the IR, or (3) new classes should be defined for use by the IR.

[sherm1]

Thanks, @clalancette! I want to emphasize @amcastro-tri's point that an "IR" is just our way of saying we want to separate parsing from making use of the parsed results. So assuming sdf provides that capability through the sdf::SDF API, that is good enough for us. We don't need to actually expose the in-memory data structure as long as we can extract what we need from it after the file has been processed. - Sherm

[clalancette]

My main reason to do the intermediate steps was to ensure that I have something that works all along the way. But if we want to go straight for an IR, I can attempt that. What are some good test cases/examples of where the IR would be helpful in the current code?

[liangfok]

What are some good test cases/examples of where the IR would be helpful in the current code?

As an example, in sdf_parser, there is a variable called pose_map that contains transforms between world-frames and body-frames. This results in the frame transformation code and XML parsing code being intertwined. An IR would help in this case by separating the parsing code from the frame computation code. I believe this will simplify the code and reduce the likelihood of bugs like those documented in #4641.

My main reason to do the intermediate steps was to ensure that I have something that works all along the way.

One way to do this is to simply not modify the existing sdf_parser and instead add new API / functionality that works alongside that provided by sdf_parser. For example, can you use libsdformat to read an SDF and instantiate a drake::systems::sensors::DepthSensor?

[jwnimmer-tri]

My advice is that the goal should be to bring in sdformat concurrently with the multibody development, and that porting drake/multibody/parsers such that RigidBodyTree is involved should not be a goal on its own -- only if it somehow makes the future work more effective. Recall that the bones of RigidBodyTree will be picked-over, but most of its code is destined for removal.

I think the good first step would be bring in sdformat as a Bazel external, with a proof-of-life unit test. Then perhaps the very first (simple) multibody dynamics tests could build the very first (simple) parser support concurrently.

Another possible first goal would be if anything in GeometryWorld #4781 wanted to have a new parser, or David's visualization work #4112 needed it for some reason.

[clalancette]

I think the good first step would be bring in sdformat as a Bazel external, with a proof-of-life unit test. Then perhaps the very first (simple) multibody dynamics tests could build the very first (simple) parser support concurrently.

All right, so I think I'll start by bringing sdformat in as a Bazel unit test. From there, I may try to instantiate a DepthSensor from SDF like @liangfok mentioned. Then I'll report back :).

Just to confirm, it looks like the long term future of Drake is to use bazel, correct? But it seems like at the moment, I also need to integrate things into cmake as well. Is that a fair assessment of the situation?

[liangfok]

Just to confirm, it looks like the long term future of Drake is to use bazel, correct? But it seems like at the moment, I also need to integrate things into cmake as well. Is that a fair assessment of the situation?

Sounds about right to me. Here is the plan: https://github.com/RobotLocomotion/drake/issues/3129#issuecomment-272572091

[amcastro-tri]

I may try to instantiate a DepthSensor from SDF

That seems pretty difficult since in it's current state a DepthSesor makes reference to a full RBT. What about instantiating a simpler object like a RigidBody? that requires parsing a <link> element, the frames that define it, its <inertial> properties.

[liangfok]

I may try to instantiate a DepthSensor from SDF

That seems pretty difficult since in it's current state a DepthSesor makes reference to a full RBT. What about instantiating a simpler object like a RigidBody? that requires parsing a element, the frames that define it, its properties.

I was thinking the existing sdf_parser.h could be used to instantiate the RigidBodyTree, and then the new libsdformat-based parser could be used to instantiate the DepthSensor using the previously-instantiated RigidBodyTree. I agree, however, that instantiating a RigidBody would be easier and would mirror existing capability within sdf_parser.h.

[amcastro-tri]

I agree, however, that it instantiating a RigidBody would be easier and would mirror existing capability within sdf_parser.h.

that is what I was thinking. Also I was trying to avoid any coupling with RBT code if possible. Instantiating a "fake" RigidBody could work also (that would be a small IR class independent of anything else within Drake right now). In that way we'd progressively move towards having a parser independent of specific Drake code that spits out an IR that anywhere in Drake can be used to instantiate anything else we'd like (RBT or whatever else).

I would suggest the following plan based on this conversation:

1. Start by bringing sdformat in as a Bazel unit test.
2. Instantiate a bare bones RigidBodyRep (representation, for the lack of better naming) class from an SDF file, with unit testing.

That bare bones class could look something like:

class RigidBodyRep {
public:
  const std::string& get_name() const;
  double get_mass() const;
  const Matrix<double, 6, 6>& get_spatial_matrix() const;
private:
  std::string name_;
  double mass_;
  Matrix<double, 6, 6> spatial_matrix_;
};

[jwnimmer-tri]

Just to confirm, it looks like the long term future of Drake is to use Bazel, correct?

Yes for the forward-looking C++ parts. (The on-the-gallows MATLAB code will probably remain in CMake-only land, until we remove it.)

But it seems like at the moment, I also need to integrate things into CMake as well. Is that a fair assessment of the situation?

If the CMake integration is easy (~1 hour) we should just do it also, so save temporary headaches. If it becomes more difficult than a couple hours, I think it would be fair to explore doing it Bazel-only. I see no reason why the ongoing System 2.0 / Multibody 2.0 ground-up rewrites couldn't be Bazel-only, and lean on the Bazel-CMake-wrapper integration work to hurry up. And if we end up needing a CMake band-aid for sdformat, we can PR it separately when the need arises.

[jwnimmer-tri]

... and as far as the first integration point, while local spike-testing against RBT could be a learning experience, I feel strongly that we shouldn't do weird hacks like RigidBodyRep or frakenbolt it onto RBT in PRs to master. We should discuss (in a more useful forum that github issue comments!) what the sdformat roadmap looks like in terms of the Multibody rewrite. Which is to say, we should let Sherm steer that ship, and only work it when the time is ripe.

[clalancette]

... and as far as the first integration point, while local spike-testing against RBT could be a learning experience, I feel strongly that we shouldn't do weird hacks like RigidBodyRep or frakenbolt it onto RBT in PRs to master. We should discuss (in a more useful forum that github issue comments!) what the sdformat roadmap looks like in terms of the Multibody rewrite. Which is to say, we should let Sherm steer that ship, and only work it when the time is ripe.

OK. Unfortunately, I'm not familiar with the Multibody rewrite. What's the idea behind it, and when is it expected to land?

In the meantime, I have gotten libsdformat integrated in with the cmake side of the build system, with a unit test (that was easier for me, since I'm more familiar with cmake than bazel). I'm working on getting it integrated with bazel. Once I have that, I'll open a pull request just to get some feedback on it. I'll then ask for further opinions on what I should do (if anything) to push this forward at the moment.

[liangfok]

Unfortunately, I'm not familiar with the Multibody rewrite. What's the idea behind it, and when is it expected to land?

Here is the issue tracking it: #4633. The development of MultibodyTree is currently in @amcastro-tri's court.

[clalancette]

OK, I've opened a pull request for getting sdformat into drake: #4831 . There are some open questions there, but at least it is a start. Please let me know what you think.

[stonier]

Closing this. The sdformat library is now accessible to Drake and updates for sdformat to handle Geometry/MBTree requirements will be dealt with on a case by case basis. @SeanCurtis-TRI will spearhead this with assistance from OSRC on the sdformat side.