Body names in contact data are insufficiently unique

[SeanCurtis-TRI]

Problem

A single sdf can be parsed into a single plant multiple times. Each time it is parsed, it is associated with a model instance name (possibly unique) and a definitely unique model instance index.

When contact is reported between bodies it is simply the body name that is referenced. So, if I have multiple instances of body Base and they each make contact with the ground, I'd get multiple contacts reported between (Base, ground). At visualization time, one cannot successfully distinguish them or separate them.

More particularly, recent advances in hydroelastic visualization (#15273) implicitly depend on having clean contact declarations. Multiple redundant contacts cause undesirable visual artifacts. While it is possible for the visualizer to attempt to accommodate this bad data, the end result would simply be wrong.

Solution

We introduce the concept of a "unique name" for a body. As a first implementation, it could simply be "model name (index)/body name". In many cases that would be overkill. If there is truly only one body with a given name, we shouldn't have to drag along all of the extra freight -- that would introduce the concept of the minimally unique name. For example, if the body name is already unique, we return the body name. If the body name is copied, but the model name is unique, it's "model name/body name". If both are duplicated, we get the full thing (with index in the name).

This would allow the contact data to meaningfully be mapped to unique bodies for unique contacts.

I've got a sequence of 4 PRs planned

• [x] #15610
• [x] #15642
• [x] #15706
• [x] #15741

The final result of the sequence can be seen in: https://github.com/SeanCurtis-TRI/drake/tree/PR_body_pair_multiple_hydro_contacts

[sherm1]

I like the minimally-unique name idea. In the worst case, would the index actually be the ModelInstanceIndex or would it count from zero among the like-named model instances? If there are two models named "kuka", say, I'm thinking it might be less astonishing to see kuka[0]/upper_arm and kuka[1]/upper_arm than kuka[4] and kuka[11] or whatever the actual model indices are.

Random thought: we could generate unique model names with less syntax by tacking a number onto the name like kuka_1 and kuka_2. You can construct bizarre cases where those aren't unique but I think they are unlikely enough to arise that we could ignore them.

[jwnimmer-tri]

According to the docs, model instance names are always unique already:

  /// @param[in] name
  ///   A string that uniquely identifies the new instance to be added to `this`
  ///   model. An exception is thrown if an instance with the same name
  ///   already exists in the model. See HasModelInstanceNamed().
  ModelInstanceIndex AddModelInstance(const std::string& name) {

[jwnimmer-tri]

... When contact is reported between bodies ...

I wasn't super clear from the write-up where these "reported" names are used. Is it only for the lcm message? I found a name there, but I didn't immediately see the name on any plant output ports. I do see that contact_results_to_lcm is creating the names per its own algorithm -- is that what you're referring to?

(When posing a problem, hyperlinks to the broken code will always help!)

If it's only LCM, then in that case I'd argue the names are GUIDs for the wire, so there is no need to trim them, just send the full information for utmost context. The display widget can trim the names if a shorter name would be clear from context when displaying.

Or, the LCM message could eschew a single string name, and pass a tuple of identifying information (e.g., model instance id + body name, or geometry id, or what have you). That might even allow the visualizer to group the information even more usefully.

[SeanCurtis-TRI]

Thanks for the comments.

model instance names are always unique already.

I'd foolishly gone looking at the parsing documentation. It is the main entry point through which most people actually end up adding model instances. And that documentation is decidedly lacking in any comment about the uniqueness of model instance names. In fact, the only documentation I did find re: parsing was this:

  /// Provides same functionality as AddModelFromFile, but instead parses the
  /// SDFormat or URDF XML data via @p file_contents with type dictated by
  /// @p file_type.
  ///
  /// @param file_contents The XML data to be parsed.
  /// @param file_type The data format; must be either "sdf" or "urdf".
  /// @param model_name The name given to the newly created instance of this
  ///   model.  If empty, the "name" attribute from the `<model>` or `<robot>`
  ///   tag will be used.
  /// @returns The instance index for the newly added model.
  /// @throws std::exception in case of errors.
  ModelInstanceIndex AddModelFromString(
      const std::string& file_contents,
      const std::string& file_type,
      const std::string& model_name = {});

This documentation strongly implies the opposite of uniqueness. If I add the model multiple times, I would not end up with unique instances -- according to the documentation. So, we clearly have some defects in that regard.

As for all of the suggestions, I'll definitely consider them. My initial proposal was quick and dirty. Other than assessing the proximal cause for visualization artifacts, I hadn't fully investigated what the best solution is. I'll have that figured out today.

[SeanCurtis-TRI]

A passing thought on the "minimally unique name".

Pros:

• Labels become as short as possible. Mindlessly always including model/body/geometry make for very large labels. Cons:
• Introduction of these labels cannot be done on visualization side. To know if a name is globally unique requires knowledge of the whole. The contact visualizer only has the knowledge given to it in a single message. It does not have a model of the world as a whole to determine if a name can be meaningfully truncated across messages.

If we produce something like a minimally unique name, it would have to be done on the lcm message generation side. It seems reasonable if the lcm message includes the full spelling and the minimally unique spelling allowing the visualizer to choose. However, my initial solution will not do this. It'll explicitly spell out the whole thing.

[amcastro-tri]

What if the LCM message included a GUID such as BodyIndex and then just the body name (or model name/body name if preferred)? would that be enough on the visualization side? Another option is just to include everything: model id, body index, geometry id, and the corresponding strings for each and let the visualizer do what it pleases with them.

[SeanCurtis-TRI]

@amcastro-tri you've basically repeated what @jwnimmer-tri proposed. My last comment still stands: the visualizer can't use a "minimally unique name" without knowing the full contents of the world.

I'm currently sending all the information to the visualizer in the message and the visualizer is using very long names for each contact. People will have a chance to experience when the PR lands.

[amcastro-tri]

Simply stating my basic understanding, if only Jeremy's idea with my simpler words. Didn't mean to offend your beautifully spelled proposal. I agree with your statement above.

[jwnimmer-tri]

I can clarify where my thinking went wrong:

For name uniqueness, I was considering the problem as "when multiple contacts are shown in viz, we need to ensure that their labels disambiguate which one is which" -- the problem is ambiguity between active contacts.

Here, you're framing the problem as "even when there is only a single contact shown in the visualizer, we need its label to inform the user about exactly which of their geometries are colliding", which is why we'd need the MbP/SG to do the shortening for us, since the viz only hears about active contacts, rather than the list of all possible geometry names.

[SeanCurtis-TRI]

even when there is only a single contact shown in the visualizer, we need its label to inform the user about exactly which of their geometries are colliding

You're heading in the right direction, but aren't quite there. For me, the visualization "name" should have a single meaning over the duration of a single simulation. The meaning of a name cannot change from one time step to the next. I don't require that the names include geometry identifiers in any way. I don't require them to include model instances in any way. If just body names is sufficient, I would favor that name as the most compact, unique identifier.

So, to recap:

• I require the names to have a single interpretation over the duration of a simulation.
• I'd like the names to be as compact as possible.

The first is easily resolved by passing the visualizer (model, body, geometry) information (which is absolutely, always a unique tuple) and it combines them. The second wish is only possible of the contact visualizer has access to the full plant's population to make determinations of "minimal" uniqueness.

[amcastro-tri]

I think that the names that Drake visualizer assigns to contacts today are compact and clear. IMO I would not change that. If we are then happy with today's way of the viz to name and show things, then we'd only require to solve the problem of non-GUID passed to the viz?

[SeanCurtis-TRI]

They are sufficiently compact and clear today so long as the names are unique. However, if your names aren't unique (like you load the same urdf/sdf multiple times or if there are multiple contacts between a single pair of bodies), a defect in visualization is revealed. The defect is indisputable. The only real question is what we do about it.

On top of that technical defect, I've added the more subtle disappointment that the visualizer cannot give consistent meaningful descriptions of the data visualized. This wouldn't stop it from visualizing all of the elements, just mean that it is passing the ambiguity on to the user.

I've probably tied those two issues too closely together: removing ambiguity for the visualizer and removing ambiguity for the user.

One thing I can do is add a configuration option such that the visualizer doesn't display the "full" names (possibly call it "compact"). This would allow sufficient functionality in 90% of the cases and keep labels short allowing the user to opt in to the full labels if they happen to be running in a context where it's necessary.

[amcastro-tri]

I've probably tied those two issues too closely together: removing ambiguity for the visualizer and removing ambiguity for the user.

As a user myself, I'd be satisfied with the fix to the "ambiguity for the visualizer". As a user, I would rarely get confused by the naming of the contact pairs since I do know my model and I can turn on/off pairs as I wish to gain even more understanding.

The second issue, "ambiguity to the user", IMO is not as much of a bug and therefore lower priority.

One thing I can do is add a configuration option such that the visualizer doesn't display the "full" names

That sounds reasonable if you'd also like to resolve the "ambiguity for the user"

[DamrongGuoy]

Does #15610 fix this issue #15555? Or we need more PRs next week to fix it?

[DamrongGuoy]

I have not followed the discussion carefully. I think now the assignee should be @SeanCurtis-TRI not @amcastro-tri , right? I see @SeanCurtis-TRI is merging code from https://github.com/SeanCurtis-TRI/drake/tree/PR_body_pair_multiple_hydro_contacts one PR after another.

We'll need it for BoD, so we'll greatly appreciate the fix. Thanks!

Assign this issue to @SeanCurtis-TRI . Please feel free to change it.

[amcastro-tri]

I'd imagine you could just pull Sean's work locally @DamrongGuoy, in case you need something ASAP for BoD. Most likely a single cherry-pick won't produce conflicts for this feature?

[DamrongGuoy]

@amcastro-tri . Thank you for the idea . Indeed I have been using Sean's branch on and off. It is very useful to have early access.

You are right that we can work around if this issue is not resolved in master before BoD.