Drake: Templating RBT requires templating parsing methods.

Created on 26 Oct 2016  路  10Comments  路  Source: RobotLocomotion/drake

Working on the first item of the checklist in #3908 (dummy templatization of RBT on <T>) I found the following problem:

URDF parsing is done with a collection of methods in parser_urdf.h/parser_urdf.cc. All these methods take as an argument a RBT. If I template RBT on <T> this implies the methods defined in parser_urdf.h become now template methods. Therefore the implementations in parser_urdf.cc will not generate objects at compile time unless I explicitly instantiate each of the methods needed.

What would be a good solution for this? possible solutions I can think of are:

  • Parsing methods in parser_urdf.h are only defined for RigidBodyTree<double>.
  • Parsing methods in parser_urdf.h are put inside an RigidBodyTreeUrdfParser<T> class templated on <T> and therefore only one explicit instantiation of this parser is needed (in contrast to explicitly instantiating every method in parser_urdf.h).

Non of these seem satisfying to me. I ask for people to contribute with ideas please.

cc'ing @sherm1, @liangfok, @jwnimmer-tri, @david-german-tri

high question

Most helpful comment

For the first spiral, I recommend modifying the URDF and SDF parsers to only work with RigidBodyTree<double>. We can then think about whether we want to fully decouple the parsers from the simulation objects through an intermediate representation like XML in a future spiral. Note related conversation in #3493.

All 10 comments

Parsing methods in parser_urdf.h are only defined for RigidBodyTree<double>.

This seems reasonable to me, given that URDF only expresses doubles. Am I missing something?

I don't think the parser should be templated at all. This is another of the many reasons we need to decouple the parser from its clients. The parsers should convert the external files into an in-memory data structure from which we can build RBPlant, RBTree, Sensors, and whatever else. We don't need to differentiate or analyze the parsers so they shouldn't be dragging our template baggage along!

@sherm1, I think your post conflates two issues:

I don't think the parser should be templated at all...We don't need to differentiate or analyze the parsers so they shouldn't be dragging our template baggage along!

I think this is the main issue, and we completely agree. @amcastro-tri's first suggestion meets that requirement. The parser can emit and operate on RigidBodyTree<double> without itself being templated on anything.

The parsers should convert the external files into an in-memory data structure from which we can build RBPlant, RBTree, Sensors, and whatever else.

This seems like an orthogonal proposal. It might or might not be useful to develop a brand-new IR, instead of parsing directly to RigidBodyTree (and other such objects). Either way, the parse output will only contain doubles, because doubles are the only scalar type represented in URDF. Thus, I don't think the IR debate should block @amcastro-tri's work on templatizing RigidBodyTree. If he updates the parsers to emit RigidBodyTree<double> now, and later they need to emit AwesomeIntermediateRepresentation instead, very little effort has been wasted.

Thanks, @david-german-tri -- I agree I had those unnecessarily entangled.

Thank you for your feedback on this. Right now in order to be able to move forward what I did is to typedef RigidBodyTree<double> RigidBodyTreed and find/replace RigidBodyTree by RigidBodyTreed in the parsing files. That works so far and I'm starting to lean towards adopting that solution.

For the first spiral, I recommend modifying the URDF and SDF parsers to only work with RigidBodyTree<double>. We can then think about whether we want to fully decouple the parsers from the simulation objects through an intermediate representation like XML in a future spiral. Note related conversation in #3493.

Just my two cents: I actually would templatize the function that parses the URDF on the scalar type, i.e. what I did here: https://github.com/tkoolen/RigidBodyDynamics.jl/blob/d018d2649ace6f77274a364dc75a7c14e1bb729f/src/parse_urdf.jl#L90

Just conceptually, double shouldn't get special treatment in my opinion. It's also slightly less convenient for the user to create a non-double version if it isn't templated. Then again, everything is 'templated by default' in Julia, so it's not as much of a hassle.

I would be OK templating the parsers on a scalar type as long as the implementation is in .cc or -inl.h files and we explicitly instantiate and document the types we support.

I see your point @tkoolen. However I have to say that I agree with the points stated by @sherm1 and @david-german-tri. I dont think it is necessary for our parsers to drag along with templated implementations since they never make use of it. That would unnecessarily complicate their coding not to mention their explicit instantiations.
I agree now that it is good enough for parsers to just emit a RigidBodyTree<double> as proposed by @liangfok (or its typedef to simplify typing).

We decided on parsers to only work on RigidBodyTree<double>. I will therefore mark this as resolved and close it. Thank you all for the helpful discussion.

Was this page helpful?
0 / 5 - 0 ratings