sdformat: Should attempt migration from model directives; if sufficient, deprecate model directives

[EricCousineau-TRI]

As follow-up from #15522. My plan is that this would happen after #15613 (and perhaps a more general version of that issue).

Suggested steps:

• Ensure that SDFormat usage in Drake can express basic construction that is (almost) equivalent to what we can express with model directives (ignoring distributions atm)
  • Will require reformulation for "leaky" scope that model directives allow vs. encapsulation, so welding "add-ons" will need to be done differently
• Possibly support drake-custom tags for defining distributions, or provide alternative design + implementation (e.g. externally define distributions)

FYI @jwnimmer-tri @ggould-tri

[aaronchongth]

After some experiments with the tests in multibody/parsing/test/process_model_directives_test, there have been some findings,

• explicitly resolving a frame by namespacing its name is not supported and not needed, hence they will be commented out,

• add_frame:

  This will explicitly resolve to the simple_model instance.

  name: simple_model::sub_added_frame_explicit X_PF: base_frame: simple_model::frame translation: [10, 20, 30]

• conversion of add_scoped_sub.yaml to add_scoped_sub.sdf is partially successful

  • tests are passing and additional comparison tests too, https://github.com/aaronchongth/drake/blob/aaron/deprecate-model-directives/multibody/parsing/test/process_model_directives_test.cc#L69-L130
  • however when parsing just add_scoped_sub.sdf in drake, the number of bodies is no longer 2, as there is the overarching model add_scoped_sub which contains the model instances simple_model and extra_model

    <model name="add_scoped_sub">
    <include>
    <name>simple_model</name>
    ...
    </include>
    <include>
    <name>extra_model</name>
    ...
    </include>
    ...
    </model>

• At the moment, it seems that sdformat is unable to replicate an unscoped include,

  • assuming add_scoped_top.sdf will contain a world instead of a model since there was a mention of a world frame
  • merge include of a model into a world is currently not supported, hence there will always be a scope of the included model

    
    # No namespace.
    # N.B. If this were added after the namespaced instances, `simple_model_origin`
    # would cause a failure because it appears in multiple model instances.

• add_frame: name: simple_model_origin X_PF: base_frame: world

• add_directives: file: package://process_model_directives_test/add_scoped_sub.yaml

• add_frame: name: simple_model::top_added_frame X_PF: base_frame: simple_model::sub_added_frame translation: [5, 10, 15]

  * with the snippet above, ideally we would like

  * but we will achieve

[aaronchongth]

The key question here would be, how critical it is to support an unscoped include into a top level world?

[EricCousineau-TRI]

explicitly resolving a frame by namespacing its name is not supported and not needed, hence they will be commented out

Sounds good!

however when parsing just add_scoped_sub.sdf in drake, the number of bodies is no longer 2, as there is the overarching model add_scoped_sub which contains the model instances simple_model and extra_model

Do you mean 2 model instances, not bodies?

At the moment, it seems that sdformat is unable to replicate an unscoped include, [...] The key question here would be, how critical it is to support an unscoped include into a top level world?

For the unscoped include, is it possible to achieve this using SDFormat's //include/@merge? https://github.com/ignitionrobotics/sdformat/pull/659

[azeey]

For the unscoped include, is it possible to achieve this using SDFormat's //include/@merge?

//include/@merge is currently only supported for //model/include, not //world/include because merging-including a model into a world is ill-defined since the model may contain links or joints, which are not valid children of //world.

[EricCousineau-TRI]

Gotcha. In this case, perhaps it's best to simply relax the world constraint (both in terms of the frame, and where it'll be included from).

[EricCousineau-TRI]

Per f2f

• ~should check for equivalent topology as if parsing into a <model/>, not <world/>~
• ~it's OK if there is an additional model instance (as long as original model instances retain same scope)~

Clarified - this is still an issue; solution would be to try and relax the constraint where //world/include/@merge is allowed if the included model's elements can live inside of //world.

Alternative is to teach Drake how to properly scope models (vs. just coloring via ModelInstanceIndex). That may be more difficult.

[aaronchongth]

Thanks for the updates!

Some other findings, I'm stuck on an error when parsing a world/model that is including another model with includes. I've created a minimal example to reproduce this, https://github.com/aaronchongth/drake/commit/004bb41af829b52febc61ac05635d5270461e369

With a model hierarchy like this

world
|__include -> weld_robots
    |__include -> simple_robot1
    |__include -> simple_robot2

It can't seem to find the models simple_robot1 or simple_robot2.

While parsing just weld_robots directly, doesn't introduce any errors, and works as expected.

weld_robots
|__ include -> simple_robot1
|__ include -> simple_robot2

test failure log: https://gist.github.com/aaronchongth/b3c641cb785030b00552d497db2b05af

I'm currently looking into it, but just thought I'd share here just in case I'm missing something or making any glaring mistakes.

Edit: https://github.com/aaronchongth/drake/blob/aaron/test-nested-include/multibody/parsing/test/process_model_directives_test.cc#L198-L213 uses the parser instead of building the package map manually, but the error still occurs. The test files are located here

[EricCousineau-TRI]

(accidental close from merge of https://github.com/ignitionrobotics/sdformat/pull/824)

[EricCousineau-TRI]

For additional discussion (from above topics and more), see Aaron's overrview in #16431

[EricCousineau-TRI]

@marcoag will be picking up on Aaron's conversion script prototype in https://github.com/aaronchongth/drake/pull/1