[multibody] Discrete MultibodyPlant uses sampled output ports

[jwnimmer-tri]

Closes 20545.

---

This change is 

[sherm1]

A minor comment: I don't think we should add new ports. A better way to deal with discrete ports (IMO) would be to add a "ForceSample()" function that would cause the ports to be recalculated.

[jwnimmer-tri]

If you have comments about the code, go ahead and place there here. Requirements questions are not effectively discussed in this medium, so please save them for elsewhere.

[amcastro-tri]

As far as I can tell parts of this do match at least my mental picture of the solution while others do not. I'll write them down here to double check with you, lots of stuff in this last single commit. The ones I believe match my mental picture are:

1. You have an addional state for ContactSolverResults (buried inside DiscreteStepMemory). I am pretty certain here you can get away with just ContactSolverResults as the additional state.
2. I like that now we can depend on the new state (memory_ticket)

The ones that do not:

1. The ability to chose a "mode". IMO this should always behave as "sampled" in your lingo. That's the one that matches the math (I'll write down separately and link here)

The ones I do not even understand:

1. Usage of a comgination of periodic and forced unrestricted updates. I am pretty certain here we can accomplish what we need with a single discrete update event. But TBH, I am not as familiar with unrestricted updates so maybe there are as good. Still I find it strange we needed two of these instead of just one.
2. As an example, body spatial accelerations output port depends on memory_ticket only. On a first read, that seems the right thing to do. What I do find difficult to reconciliate is how to handle parameters. Say someone does change a paramter, how does the invalidation go? A user won't see the effect of parameter change untile the next discrete update step. Maybe that's the right thing, but I wanted to double check your thoughts on this.

[jwnimmer-tri]

I will comment though that it seems to me that most of the complexities introduced in this PR stems from trying to keep the duplicated ports.

The complexity isn't from the new "unsampled"-named ports alone. If we allow SetUseSampledOutputPorts as public API, or if we add "foo_unsampled" output ports, or both -- then in those cases we need the template <bool sampled> on calc functions. Only if we provide no mechanism whatsoever for feedthrough output do we get to shave the code down a little bit.

Our agreement with Russ from this spring / last fall about adding more State to MbP was that we need an opt-out so that planning and analysis code is not polluted by the extra memory. The default can be memory (since that is the primary use case), but we need an opt-in way to not add more State. My proposal is that SetUseSampledOutputPorts(false) will be that opt-out, and so we need to implement a feedthrough CalcFooOutput<sampled=false> one way or another.

In any case, the current hard work of making a Memory-only implementation of the Calc functions remains no matter what, so for now I'm still going to focus on that (and landing the preparatory cleanups so that the Memory work is crisp and clear).

[jwnimmer-tri]

I don't think the sampled ports should reflect the change in parameters instantaneously.

I could probably go either way on that. I agree with Xuchen that it would be OK to say "sampled output ports never reflect changes to parameters until after the next sample". However, I would also be OK saying "sampled output ports do not necessarily reflect changes to parameters until after the next sample". The entire premise of a Parameter is that it's not time-varying, so how it interacts with time stepping is somewhat of a moot point. If it's easy to implement "never" then we should do that, to that it's simple to document and test. If it's easier to implement "some parameters might leak out, even without a step" then that might be okay too.

[jwnimmer-tri]

... need to implement a feedthrough CalcFooOutput<sampled=false> one way or another.

Hmm, I wonder -- if sampling was turned off in the Russ example, maybe the Memory is just a cache entry? You could imagine that if someone asks for an output port value from an unsampled discrete plant, the right thing to do is to calculate the unrestricted update event into a temporary, and call the calc output function on that scratch Memory. I might try that in my code later on.

[jwnimmer-tri]

Not sure if I'm following here. If sampling is turned off, then "memory" isn't necessary at all for any computation; i.e. any computation result can be recovered from the input port values and the discrete states.

The idea would that that for CalcFooOutput (for dynamics output ports), the first thing it does it obtain const Memory& memory ... somehow -- when sampled, get it from State; when unsampled, evaluate the cache entry. Now, implement the Calc code in terms of const Memory& memory instead of const Context& context. The important logic inside of CalcFooOutput is the same now , no more extra branching. I could probably even declare the member function to take const Memory& only -- no Context -- and use the port-declaration sugar to decide how the Memory reaches it, with a few little shims.

[jwnimmer-tri]

Hmm, even after rebasing away the 800 files thing, the review page is still pretty slow. I'll probably need to open a new PR number. I'll wait until a couple more cleanups land before I do that.

[jwnimmer-tri]

Okay, unfortunately it's time to close this PR number down and get a fresh one. The temporary 800 files in the review has clogged things up. I'll open a new PR number (with the same branch name) shortly, and edit this comment to point there.

=> https://github.com/RobotLocomotion/drake/pull/21623