RobotLocomotion / drake

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

EvalUnrestrictedUpdate() should be able to terminate simulation #4447

Closed sherm1 closed 2 years ago

sherm1 commented 7 years ago

PR #4382 introduces a general-purpose event handler that can update any state variables. It should also be able to halt the simulation (end condition reached, robot fell down, sanity check failed, etc.).

This can be as simple as adding a bool functional return.

RussTedrake commented 6 years ago

+1 for this. It would resolve my issues of the zero-crossing detection failing on the zeno solutions of #8443.

sherm1 commented 6 years ago

What do you think, @edrumwri -- this seems to have bubbled up finally!

jwnimmer-tri commented 6 years ago

What about just using a publish event "I'm done"? Publish events are the supported idiom to expose information from within a simulation to without.

RussTedrake commented 6 years ago

@jwnimmer-tri -- do we then need a mechanism for the Simulator to catch that event?

sherm1 commented 6 years ago

I do like the idea of a Termination event -- we actually have an Initialization event and they seem like a natural pair. I have to say I haven't had much luck with that in real life though, Simbody used a sim-killer bool initially and then a decade later we were still waiting for a reason to do something fancier.

jwnimmer-tri commented 6 years ago

I guess I am very skeptical about binding control-flow or owns-the-clock semantics into the Simulator's timestepper, in a way that Systems can presume anything about it.

I would imagine more like this:

SystemThatKnowsWhenItsDone system;
Simulator simulator(&system);
simulation.StepUntil([&](const Context& context) {
  return system.is_done(context);
});

or possibly with some simulator.set_termination_condition(callback) to pass it separately from the StepTo call.

Basically, the application frobs the simulator to tell is when to stop. The thing being simulated can't assume that it will be able to halt time!

sherm1 commented 6 years ago

That scheme would be tricky to square with variable step integration, because trial steps might appear to trigger termination, but might not actually when a more sensible step is taken. We already have a great mechanism for dealing with that -- witness functions (zero crossing detection). Those deal nicely with apparent events that might not actually occur, because they are integrated into the step size choice algorithm which is obligated to isolate the moment where the event actually occurs. One could easily write a system.is_done(context) witness function that changes sign when the system can't go on. The simulator would automatically isolate the exact termination time and ignore spurious ones.

An advantage of terminating from an event handler is that once you get there, you know you are looking at a Context that is part of the actual trajectory and not a trial. So you can check for fatal conditions reliably. And since the Simulator makes an explicit call to the event handler (always between time steps) it is in a good position to decide not to take any more.

jwnimmer-tri commented 6 years ago

In the case of my callback, I was imagining it would only be queried once a step is retired -- i.e., nothing speculative. Along the lines of PublishEveryTimestep or similar, where it happens after all of the mutations, but before time advances again.

I was imagining that isolating the termination to a small epsilon was not required or interesting, but that rather the "decide to stop" was more of a user convenience. As in, "My robot has reached an end state, stop recording a video". Indeed, if this were for some kind of analysis or numerical purpose (perhaps as part of a larger experiment) we'd want to use the real events system and witness and everything in order to make it precise.

So maybe we are both right? If someone wants to model some kind of state-machine event, then that's within the framework. If they just want some sugar instead of writing this out ...

SystemThatKnowsWhenItsDone system;
Simulator simulator(&system);
const auto& context = simulator.get_context();
while (!system.is_done(context)) {
  simulation.StepDt(0.001);
}

... then a simple predicate callback can help.

sherm1 commented 6 years ago

Also, it's not obvious to me how to implement a sensible Termination Event. The standard event paradigm is: event occurs -> handler invoked. There are two awkward bits here:

Say you want to terminate when the robot falls down. That seems more naturally expressed as a "falling down event" (COM goes below some specified height, e.g.) that triggers termination. An easy way to write that is to create a witness function that goes from positive to negative as the COM drops. That's a perfectly ordinary event and can trigger any of our existing handlers. Then the handler can look at the Context, and decide whether termination is warranted. The only missing piece there is that we don't have a "must terminate" return from our handlers.

sherm1 commented 6 years ago

(Sorry, our comments crossed in the mail.) I like the loop you wrote -- that would work now, I think. In Russ's case (as I understand it) he wants to terminate due to detecting Zeno's paradox problems where he finds the same event over and over. That's probably most easily expressed in the event handler (where it can check a mutable counter or whatever). With some infrastructure it could also be done with a system.is_done(context) but that would mean putting the counter in the context (an abstract state would work). The handler would bump the counter on every call; the system would die if the counter got too big. Or ... just have the handler report directly that it has had enough.

edrumwri commented 6 years ago

Sweet- I should wait to reply more often since you guys are so quick to discuss the possible technical approaches.


Evan Drumwright Senior Research Scientist http://positronicslab.github.io Toyota Research Institute Palo Alto, CA

On Fri, Mar 23, 2018 at 5:38 PM, Michael Sherman notifications@github.com wrote:

(Sorry, our comments crossed in the mail.) I like the loop you wrote -- that would work now, I think. In Russ's case (as I understand it) he wants to terminate due to detecting Zeno's paradox problems where he finds the same event over and over. That's probably most easily expressed in the event handler (where it can check a mutable counter or whatever). With some infrastructure it could also be done with a system.is_done(context) but that would mean putting the counter in the context (an abstract state would work). The handler would bump the counter on every call; the system would die if the counter got too big. Or ... just have the handler report directly that it has had enough.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RobotLocomotion/drake/issues/4447#issuecomment-375832630, or mute the thread https://github.com/notifications/unsubscribe-auth/ACHwz8ADWx5-GEaC-v8qmlKPLz545OQbks5thZWNgaJpZM4LJhQ9 .

--


Confidential or protected information may be contained in this email and/or attachment. Unless otherwise marked, all TRI email communications are considered "PROTECTED" and should not be shared or distributed. Thank you.

RussTedrake commented 6 years ago

Just one thought to add -- a reason for me wanting to terminate this particular simulation is that it currently is not possible to simulate the dynamical system accurately beyond these events. (My passive walker enters zeno, and then eventually falls through the ground regardless of the accuracy I set for the zero-crossings). So terminating in this case is (somewhat) required, and terminating on the event instead of at the next 0.01 timestep might be a notable difference.

RussTedrake commented 6 years ago

I also dislike the idea of imposing a timestep (0.001 in your example above) in the outer loop of the simulation. There is no reason to impose that the timestep takes a major step on any particular period like that (regardless of how big or small, we can find an example where it's a bad idea).

edrumwri commented 6 years ago

Russ, do you have a timeframe on when you need this addressed? Can this go into my queue or does it need to go onto my stack?


Evan Drumwright Senior Research Scientist http://positronicslab.github.io Toyota Research Institute Palo Alto, CA

On Sat, Mar 24, 2018 at 9:51 AM, Russ Tedrake notifications@github.com wrote:

I also dislike the idea of imposing a timestep (0.001 in your example above) in the outer loop of the simulation. There is no reason to impose that the timestep takes a major step on any particular period like that (regardless of how big or small, we can find an example where it's a bad idea).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RobotLocomotion/drake/issues/4447#issuecomment-375907340, or mute the thread https://github.com/notifications/unsubscribe-auth/ACHwz5RlqrvNaxkoPITPIfed1WuaMYW2ks5thnmGgaJpZM4LJhQ9 .

--


Confidential or protected information may be contained in this email and/or attachment. Unless otherwise marked, all TRI email communications are considered "PROTECTED" and should not be shared or distributed. Thank you.

jwnimmer-tri commented 6 years ago

If the problem is that a System can no longer numerically compute its dynamics, it seems like we should express that to the framework -- either a "stopped" mode variable that a witness sets, and then its CalcTimeDerivatives can report zero, or else a way for the System to express failure to the Simulator. I am still extremely skeptical that having a System be able to declare "I know everything there is to know about the entire world, no matter what Diagram I'm embedded in, and I know for a fact that everyone's time forevermore needs to stop". To me, that still seems to defeat the premise of how a System should be allowed to operate.

sherm1 commented 6 years ago

(long pause for firefighting) I think it is better to think of a Diagram as a single entity composed of cooperating subcomponents, assembled intentionally by a cognizant engineer. All the components have to work in order for the Diagram to function, so if any individual leaf says it can't continue, then no one can. @edrumwri drew an apt analogy with errors in a program -- we happily allow low-level components to murder whole processes by throwing exceptions or aborting when they can't go on.

There are of course other models for sets of loosely-coupled components that can recover nicely from isolated failures, but our Diagram architecture isn't well-suited to that (in contrast we could easily do that with LCM-interconnected components). I'm not sure we have a use case requiring fault-tolerance within a Diagram, and there are plenty of use cases for having one leaf system call a halt. For example, it is easy to make a small "monitor" system that watches some plant output (say height of COM) and halts the simulation when the COM drops below a pre-set level.

On a pragmatic note it would be very easy to add a return status to our handlers so that at least the handler can convey that it can't proceed. For now we can have the Simulator terminate the whole simulation but that still would permit a more subtle response later.

sherm1 commented 6 years ago

Recording notes from conversation with @edrumwri:

pvarin commented 6 years ago

I'm about to start some reinforcement learning work that I'd like to use the drake simulator for. I need to be able to terminate the simulation when a goal is met or a failure occurs (e.g. @sherm1 's earlier comment about falling down). I also don't need super accurate resolution on the time of the event. A witness function that can terminate the simulation seems like it would work perfectly.

tri-ltyyu commented 5 years ago

Sherm to investigate. Problem showing up in manipulation station example.

tri-ltyyu commented 5 years ago

Original solution is too invasive. Looking at alternative of adding event status to callback.

tri-ltyyu commented 5 years ago

There're other higher priority work than this. Re-visiting in Q2. Waiting for #11059 to happen first.

RussTedrake commented 5 years ago

Not complaining, simply noting that I have yet another case where I was wishing for this (plotting the return map for the Spring Loaded Inverted Pendulum by running many simulations from apex to apex -- apex defined by a witness function).

sherm1 commented 5 years ago

Good -- thanks for the encouragement! A good candidate for Q2 OKRs.

RussTedrake commented 5 years ago

fwiw -- might it be cleaner now to think of this as a termination "event" that could be triggered? Not sure if that's really better or worse that some method that could be called in an unrestricted update, but it seems clean.

sherm1 commented 5 years ago

Better I think to allow any event handler to request termination. For example, in your rimless wheel, the termination condition is detected by the same reset/handler functions that are used during execution. The trigger is the usual witness, but the response differs due to conditions observed in the handler.

RussTedrake commented 5 years ago

that's fine for me.

mpetersen94 commented 5 years ago

Bumping this as it would be helpful for the work I am doing right now. Any plan on when this will be added?

sherm1 commented 5 years ago

@mpetersen94 this has been on hold for a while I think due to lack of demand. Good to hear it would be useful for you! I will make sure it gets onto our next 3-month plan.

jwnimmer-tri commented 5 years ago

@mpetersen94 Is your use case more like the above cases of (1) return Status::FAILURE -- "Some specific system has failed and can no longer compute its dynamics" or, (2) simulation.StepUntil([&](const Context& context) { return system.is_done(context); }); --"No need to keep simulating once a final constraint has been achieved"?

Or I guess in general, what is the flavor of your use case?

mpetersen94 commented 5 years ago

Mine is more of flavor 2. I want the simulation to continue until some task is accomplished and then stop. I'm using it to collect a series of trajectories followed by a controller under different initial conditions so I don't know how long each case will take to achieve equilibrium.

sherm1 commented 5 years ago

@mpetersen94 how were you planning to express that as an event?

mpetersen94 commented 5 years ago

I would like to have the controller system detect when the input is within some threshold and then notifying the simulator it can stop (or whatever other way of telling the simulation to stop is added to drake).

edrumwri commented 5 years ago

@mpetersen94 please take a look at the witness functions and see whether that construct will allow you to express this condition.

mpetersen94 commented 5 years ago

That seems like what I would need. Is there a good example of using this to terminate a simulation?

edrumwri commented 5 years ago

There is not (yet), because the last little bit of wiring has yet to be connected into the Simulator. It wouldn't be too hard for us to implement that, I think.

Before we go further with this, though, would it be feasible for you to just call AdvanceTo(.) in fixed increments until the condition is met?

mpetersen94 commented 5 years ago

I guess that is the hacky way to accomplish what I want. Make a step with AdvanceTo(.), get the context out, check the state, repeat. That should work. And I don't have to figure out witness functions. Thanks!

edrumwri commented 5 years ago

Yeah, it's hacky but you get a solution immediately as opposed to whenever we can get a PR through!

sherm1 commented 4 years ago

@mpetersen94 @pvarin @RussTedrake (regarding early termination of Drake simulations as discussed above): please see the new Simulator monitor() function added in PR #12213. Documentation is here. That's not the general solution to returning status from event handlers, but I think it might be enough for the use cases you mentioned above. Is it?

mpetersen94 commented 4 years ago

That seems to cover my use case. I'll probably need bindings but that is a separate and relatively straight forward issue.

jwnimmer-tri commented 2 years ago

The set_monitor is bound in pydrake by now.

Is the conclusion here that set_monitor is sufficient to meet the needs? In which case, time to close this issue? Or if not, can someone clarify what missing feature or use case still remains?

sherm1 commented 2 years ago

This is still on my todo list. set_monitor() is a good workaround in practice but isn't the general solution I want, especially for continuous (variable-step) simulation because it is limited to step boundaries. It also requires someone to have a godlike perspective on the diagram, reducing modularity. (That is, you can't currently write a leaf system that can guard itself when inserted naively into a diagram.) Internally we are only doing discrete simulation but we have plans to fix up continuous next year.

jwnimmer-tri commented 2 years ago

Hmm, so is this just a duplicate of #13799, in that case?

sherm1 commented 2 years ago

Basically yes -- that one has some more implementation detail and references this one. We don't need them both open.