Wanted: EvalDiscreteVariableUpdates

[RussTedrake]

The recommendation now is to not call CalcTimeDerivatives, but to instead call EvalTimeDerivatives. But there is no comparable API for discrete updates; I think we still call CalcDiscreteVariableUpdates. The lack of parallelism is jarring.

[sherm1]

Hmmm. This is much trickier than it sounds. CalcDiscreteVariableUpdates() is mostly useless at this point because it generates "Forced Event" triggers and then invokes the discrete variable event dispatcher. Unless the dispatcher has been overridden (not recommended) that's essentially a no-op. Most of our discrete event handlers respond to periodic event triggers (potentially with many different periods in a Diagram). (Per-step, witness function, etc. triggers can also cause discrete variable updates.) Users could make discrete event handlers respond to forced event triggers also, but rarely do.

Probably a better approach here would be to deprecate CalcDiscreteVariableUpdates() or at least rename it to something less tempting like IssueForcedTriggersForDiscreteUpdates().

Any thoughts @edrumwri ?

[edrumwri]

The periodic triggering is good for our hybrid simulation framework, though I could imagine one building a discrete system and just wanting it to iterate through calculating the discrete variable updates. Though that would take some additional work to make that application smooth.

We'd need to consider how to readily make discrete systems respond to forced updates (i.e., rather than making users declare two kinds of triggers), for example.

I'd be curious to hear what Russ thinks about intended uses. Some polishing is in order, but it's yet unclear that EvalDiscreteVariableUpdates(.) is what we want.

---

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

On Wed, Mar 13, 2019 at 9:52 AM Michael Sherman [email protected] wrote:

Hmmm. This is much trickier than it sounds. CalcDiscreteVariableUpdates() is mostly useless at this point because it generates "Forced Event" triggers and then invokes the discrete variable event dispatcher. Unless the dispatcher has been overridden (not recommended) that's essentially a no-op. Most of our discrete event handlers respond to periodic event triggers (potentially with many different periods in a Diagram). (Per-step, witness function, etc. triggers can also cause discrete variable updates.) Users could make discrete event handlers respond to forced event triggers also, but rarely do.

Probably a better approach here would be to deprecate CalcDiscreteVariableUpdates() or at least rename it to something less tempting like IssueForcedTriggersForDiscreteUpdates().

Any thoughts @edrumwri https://github.com/edrumwri ?

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

--

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.

[avalenzu]

The intended use here is not for simulation. It's for trajectory optimization (at least that's why I want it). Consider this portion of the dynamics constraint in systems::trajectory_optimization::DirectTranscription: https://github.com/RobotLocomotion/drake/blob/82ae060eea9c2f1e6b9fa79c8574e1464d6dff8e/systems/trajectory_optimization/direct_transcription.cc#L83-L86

[sherm1]

@edrumwri and I currently favor the following solution to the problem of advancing a discrete system outside of Simulator (by force-triggering its event handlers):

• whenever a System declares an event (trigger, handler) pair, also register (forced, handler).

This will have no effect under normal simulator operation, but manual triggering of forced events (which is what CalcDiscreteVariableUpdates() actually does) will cause all the handlers to execute. A Diagram that has been engineered to operate like a simple discrete system will have only a single periodic-triggered handler, or several handlers on the same period, so will behave as expected. Diagrams with many handlers with a variety of triggers will get them all executed, which is the same behavior we're currently getting with the common-but-discouraged overloading of the DoCalcDiscreteVariableUpdates() dispatcher.

Thoughts? attn: @RussTedrake @avalenzu

[sherm1]

(Note: @edrumwri, if we implement this I think we should not generate multiple forced events for the same handler.)

[edrumwri]

A few points of clarification, per f2f discussion with @sherm1:

1. When Sherm says "we should not generate multiple forced events for the same handler", that is referring to the scenario like:

MySystem() {
   this->DeclarePerStepPublishEvent(&MySystem::HandlePublish);
   this->DeclarePeriodicPublishEvent(period, offset, &MySystem::HandlePublish);
}

We do not want MySystem::Publish() to call MySystem::HandlePublish() twice (which it would if we followed Sherm's initial prescription verbatim).

2. User-declaration of force-triggered events would still be supported.

[avalenzu]

Would we then continue calling CalcDiscreteVariableUpdates() to advance the system? Or would there be an additional EvalDiscreteVariableUpdates() method?

[sherm1]

CalcDiscreteVariableUpdates() would work as before. We can also add EvalDiscreteVariableUpdates() (which would return a reference to a cached version of the usual output from CalcDiscreteVariableUpdates()), although that's a separate issue. It's not immediately clear to me what the advantage would be of caching that computation though -- typically you want that computation performed due to an external event, not due to changing inputs. How would you envision that working?

[avalenzu]

I was mostly thinking of the lack of parallelism that Russ mentioned in the original post.

[sherm1]

@avalenzu, I'm not sure I see the parallelism yet (except for the names). Possibly I'm not understanding the intended use. Let me see if I can formulate my confusion:

• Say we have a context containing the state at step n, xₙ. Call that contextₙ
• The Calc method is essentially xₙ₊₁ ← Calc(contextₙ), that is, unconditionally compute xₙ₊₁ from xₙ. Presumably the application then does something with xₙ₊₁.

An Eval method would work as follows (by analogy with all other Eval methods):

• the context would contain a cache entry x̂ the same size as (discrete) state x.
• Eval(contextₙ) means:
  • if x̂ is out of date with respect to the given context, update it with x̂ ← Calc(contextₙ)
  • return a reference to x̂
• the application would then do something with x̂.

What's unclear to me is the meaning of "out of date" in this case. Normally we don't update discrete variables because they are "out of date"; we update them when it's time to move from n to n+1. So what would cause EvalDiscreteVariableUpdates() to actually call Calc() in the application you have in mind?

[avalenzu]

Sorry for the confusion. I think (correct me if I'm wrong, @RussTedrake) that the original complaint of missing parallelism was entirely about the names.

Personally, I don't have any objection to only having CalcDiscreteVariableUpdates().

[jwnimmer-tri]

The point of EvalDiscreteVariableUpdates is not about caching. As noted above, typically each time we call it the state will be different so it will not re-use any prior computations. (It's prereq ticket will be "all sources".)

The point is that it will use pre-allocated storage, and will have a return value.

The output-argument form of CalcDiscreteVariableUpdates is difficult to use, and extremely difficult to explain to new users. A simple function with a simple return value will be valuable sugar.

[sherm1]

I like having the cache memory there -- that's a feature in the #9171 design also.

However, it isn't clear to me what EvalDiscreteVariableUpdates() would do. If it is only a wrapper around the mostly-useless CalcDiscreteVariableUpdates() that deals only in Forced Events, that's not much help. To be useful it needs to do something more meaningful, or be coupled with hidden Forced triggers for all events (or just some?). We could make it trigger every discrete variable update event, with the caveat that that won't necessarily be what the Simulator will do next since those events might not actually be scheduled to trigger. Are there coherent semantics for this method that would be meaningful to users?

[jwnimmer-tri]

I think triggering all discrete update events no matter the trigger (and saying that it will do so) would suffice.

Users can check GetUniquePeriodicDiscreteUpdateAttribute first if they want to be careful (we could mention that in the docs).

It would probably also be fine for EvalDiscreteVariableUpdates to throw if the GetUniquePeriodicDiscreteUpdateAttribute does not exist.

[sherm1]

That's reasonable, but if we do it for Eval then Calc should have the same meaning. How bad would it be to change Calc's behavior? Odd things would happen in cases where parallel Forced events have been defined (like multiple updates). My thought would be to start by renaming the old method to better reflect its odd semantics, then introduce the better behaved methods using either the original names or some new ones.

[jwnimmer-tri]

I agree (per #10915 discussion) that CalcDiscreteVariableUpdates 2-arg should be burned to the ground. Once we do that, having a function EvalDiscreteVariableUpdates would not have any impedance mismatch.

If you decide to deprecate-and-rename CalcDiscreteVariableUpdates 2-arg rather than deprecate-for-removal, I think you can choose a sufficiently long and awkward name for it that parity with EvalDiscreateVariableUpdates would not be an issue.

Or were you saying that every Eval function must have a same-named Calc function, as a twin? I don't think that's true. I'd be happy to have Eval only. If the user needs a local copy of the result, they can clone it.

[sherm1]

Or were you saying that every Eval function must have a same-named Calc function, as a twin?

Well there is always a Calc method backing up every cache entry, but whether we expose it publicly is a different question. I'd be happy not to do so unless there is a good reason.

[sherm1]

With the name EvalDiscreteVariableUpdates() I think a user would be surprised to find that this did not necessarily compute the next-step update that would be performed by the Simulator. That is, it doesn't compute x(n+1) from x(n) unless the Simulator would also determine that every one of the discrete update events did actually trigger. In simple cases with just one discrete update event with a simple trigger condition, the user might get the answer they were hoping for. But applied to a blind Diagram, that seems doubtful.

Since we don't have a Simulator in this use case, we can't ask it for help. My inclination would be to name this function to better capture what it is actually doing, which is calculating the effect of simultaneously triggering all the discrete update events applied to the current discrete state, with the result going into a cache entry that is not part of the normal Simulator workflow.

First try: EvalEffectOfTriggeringAllDiscreteVariableEvents() ?

[jwnimmer-tri]

If that's what you're worried about, my vote would be to keep a simple name but have it throw when there are staggered events.

Maybe EvalUniquePeriodicDiscreteUpdates? Which would throw the same as GetUniquePeriodicDiscreteUpdateAttribute.

[RussTedrake]

As discussed in #17824, I'm marking this as high priority. Many of our basic algorithms on Systems (like Linearize) are giving the wrong answer for systems that use the new Drake event handling. I think it is extremely important that we fix it.

[sherm1]

I'm working on this today.