The default "point" contact model currently ignores hydroelastics

[sherm1]

Per Slack thread here a user was understandably surprised that after properly annotating geometry with hydroelastic properties his simulation used only point contact.

Changing the default contact model from Point to HydroelasticWithFallback would eliminate that surprise but still use point contact for un-annotated geometry.

If we make this change we should update the hydroelastic user guide also to reflect it.

My proposal would be to change it now for Drake 1.0. Assigned to Rico initially for delegation/disposition.

[rpoyner-tri]

I seem to recall something in the in-progress stability guide regarding whether defaults changes are breaking changes. I don't know if this change would break stuff, but I wouldn't be entirely surprised if it did.

Any thoughts about the likelihood of breakage: @DamrongGuoy @joemasterjohn ?

[joemasterjohn]

I would say that it is most definitely a breaking change. If a user had a model with <drake:compliant_hydroelastic/> defined yet used the default point contact mode (purposefully or inadvertently) would see big changes in their simulation.

[jwnimmer-tri]

We should definitely announce it carefully, because it might impact users, but recall that anything to do with hydroelastics is categorized as "experimental" and is subject to breaking changes without a deprecation period. If we think the new default is better, we should land it, with suitable announcement text.

[rpoyner-tri]

@DamrongGuoy agreed to evaluate the change over drake and anzu, and help understand the impact.

[DamrongGuoy]

@sherm1 @rpoyner-tri
As the assignee of this issue, I tested the Draft PR #16569. It already triggered 69 unit test failures (out of 788) in multibody. I guess it won't be trivial to switch.

damrongguoy@P52-DGuoy:~/GitHub/DamrongGuoy/drake (default_HydrolasticWithFallback)$ bazel test //multibody/...
...
Executed 788 out of 788 tests: 719 tests pass and 69 fail locally.
INFO: Build completed, 69 tests FAILED, 1270 total actions

CC: @amcastro-tri

EDIT: I take it back. All the 69 unit test failures came from the same one line that I can fix easily, I think.

[jwnimmer-tri]

Fixing the plant's own unit tests isn't really the question here. We should expect that any change to MbP defaults means that we need to carefully audit the MbP's personal unit tests to make sure that they are still testing what they were supposed to be testing. Specific contact-related tests might need to opt-in back to a different mode flag. This will be similar to when we switch from TAMSI to SAP, or anything else. The plant's own unit test folder will require specific review.

The real question here is impact for users -- whether any code beyond the plant's unit tests starts behaving incorrectly.

[DamrongGuoy]

I agree that a few lines of change in multibody could have a significant impact on the user's code. I'm testing Anzu-master-Drake-PR now. I wonder what I should do next.

I'll need to talk to users more, I guess.

I wish we have some kind of Quality Assurance department. :thinking:

[sherm1]

Damrong, can you explain what caused the MbP failures? I was thinking HydroWithFallback would be exactly equivalent to Point contact unless there was specifically-tagged hydro geometry. That seems unlikely in MbP tests unless they were specifically hydro tests or maybe tests that the model-selection feature is working properly (with those tests assuming point contact as the default).

[jwnimmer-tri]

@sherm1 If you look at the PR's pushes, I think the explanation is pretty clear.

[DamrongGuoy]

Sorry for the false alarms. The unit test failures in Drake are easy to fix. It's like there are two places in the code where we need to flip the two switches at the same time.

There is no more unit test failure in Drake #16569. I do have one unit test failure in Anzu that I need to check. Hopefully, it's just another simple fix.

[DamrongGuoy]

@Brian-Acosta Thanks for your question about plant.set_contact_model(). This issue considers changing the default to hydroelastic with fallback. You might want to attend the coming Drake Developers meeting on 2/14/22 this Valentine Monday.

Now we have a draft Drake PR https://github.com/RobotLocomotion/drake/pull/16569 for this issue.

[DamrongGuoy]

Update on Anzu side, I got one failed unit test with easy fix.

A simple fix (back to the past) is to restore the point contact:

builder.mutable_builder().mutable_plant().set_contact_model(
    drake::multibody::ContactModel::kPoint);

A better fix (step into the future) is to improve definitions of collision hydroelastic geometries and to remove the many tiny collision spheres. Here I omit details from Anzu since it's not a part of Drake.

This raises an interesting question in hydroelastic adoption. Users might already have many models with custom collision spheres for point contacts. Switching the contact model default might trigger failures in their code.

We'll have to tell them to do set_contact_model(drake::multibody::ContactModel::kPoint); to restore the glorious past, while we're stepping into the bright future. CC: @calderpg-tri

[jwnimmer-tri]

I got on the phone with Damrong. Something about the prior post is confused or incomplete. He'll correct it later on.

[DamrongGuoy]

Thank you @jwnimmer-tri . I think that the unit test was set up with collision geometries for both point contact and hydroelastics. In the past, by default, we use point contacts, so the hydroelastic parts didn't take effect. In the future, if we default to hydroelastics, the hydroelastic contact shows up and changed the behavior of the sim.

If users never set <drake:rigid_hydroelastic>,<drake:compliant_hydroelastic> tags before, the behavior should be the same for the new default. I think that's why most of the unit tests did pass. Only this one unit test failed.

@calderpg-tri could you confirm what I said about mixing collision geometries with point-contact + hydroelastic-contact in that unit test, please?

[calderpg-tri]

Yes, the issue is a model in Anzu that mixes collision geometries using point-contact and hydroelastic-contact. I believe the ideal solution would be to return to the original model (just using boxes and cylinders, no additional collision spheres or tweaks) with all parts using hydroelastic contact.

[DamrongGuoy]

@sherm1 #16569 has merged. If it passes Nightly CI tonight, could you please close this issue tomorrow?

[sherm1]

I'll close it now -- we can reopen if it fails!