search

RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.29k stars 1.26k forks source link

Initializing a MultibodyPlant segfaults when time_step=0 #13678

Closed deq2 closed 4 years ago

deq2 commented 4 years ago

I'm trying to use direct collocation with a custom urdf robot. To do this I need to initialize the the MultibodyPlant with time_step=0.0. However, the program fails with a segfault. I've added a code snippit below: https://gist.github.com/deq2/b39bf8d1f8a1108de000ead2ac90f0f6

(Note I was running with Python version Python 3.6.9 on Ubuntu 16.04)

rpoyner-tri commented 4 years ago

I've adapted the snippet to live in the drake tree in my branch; seg-fault reproduced. Investigating.

sherm1 commented 4 years ago

Looks like it declares the MultibodyPlant to be continuous but then asks (at the end) for the discrete state variables (which don't exist in that case). I believe if it asked for continuous state variables all would be well.

rpoyner-tri commented 4 years ago

@sherm1 Agreed. I'm pondering whether the API hazard with discrete state is sufficiently documented.

rpoyner-tri commented 4 years ago

Ah. The code path that yields the seg-fault runs through a DRAKE_ASSERT that is apparently disarmed in libdrake.so as built. Do we provide an asserts-enabled version of libdrake.so with instructions, to help external users debug this sort of thing?

rpoyner-tri commented 4 years ago

@deq2 this branch https://github.com/rpoyner-tri/drake/tree/gh13678 has a couple of commits showing the adapted code, and some changes I made to avoid the misuse and print out various bits of context info.

sherm1 commented 4 years ago

The code path that yields the seg-fault runs through a DRAKE_ASSERT ... Do we provide an asserts-enabled version of libdrake.so ...

That should instead be a nice error message like "You asked for a reference to discrete variables but there aren't any in this model." Minor user misunderstandings shouldn't cause segfaults!

rpoyner-tri commented 4 years ago

@sherm1 again, agreed. Presumably that's just a change of error handling. I'll look at the options.

deq2 commented 4 years ago

Ah I understand the issue. Thanks for getting back to me so quickly.

rpoyner-tri commented 4 years ago

@deq2 glad we could help. I'll have a PR up at some point for an improved error message, but that shouldn't hold you up.

rpoyner-tri commented 4 years ago

https://github.com/robotlocomotion/drake/pull/13681 is one proposal for a fix.

rpoyner-tri commented 4 years ago

Patch merged.