Drake: Add an OutputPort implementation that doesn't own its data

[david-german-tri]

Useful for zero-copy reference to input, state, or cache lines. Low priority because it's a performance optimization with no impact on the API that System<T> provides.

[amcastro-tri]

I am curious about the statement about that the API for System will not change. How would the following line for PR #3132 read once this is implemented?:

output_vector->get_mutable_value() = input_vector->get_value();

would it still read the same without incurring in a copy?

[david-german-tri]

would it still read the same without incurring in a copy?

No, but that's not part of the API that System<T> provides; it's an API that System<T> uses. It is true that Systems written prior to this change will require changes to take advantage of zero-copy.

[sherm1]

In issue #2721 we settled on "Eval" specifically so we could have a uniform convention for the "cacheable value" pattern that everyone can follow without thinking -- that makes the API simpler to understand. The general pattern for an "Eval" method should be roughly:

const ThingType& EvalThing(const Context& context) const { 
  auto& entry = context.get_cache_entry<ThingType>(thing_cache_ticket_);
  if (!entry.is_valid())
    CalcThing(context, entry.get_mutable_value());
  return entry.get_value();
}

In that sketch, CalcThing() has the signature that is currently being used by EvalOutputPort(). I propose to rename EvalOutputPort() to CalcOutputPort(), then implement an EvalOutputPort() that follows the canonical pattern. Then it can return a reference to its value in the Context, which might be a cache entry, state variables, or a "pass through" input port value. Copying is then eliminated by construction.

Calc methods could be private, but are likely to have separate utility for performing calculations without affecting the cache.

[amcastro-tri]

I really like having the "Calc" prefix on methods to denote those that actually do perform the computation.
Also it'd seem that CalcThing() would then look closer to a simple computational routine for which the user only has to focus on the math? (I mean, with not a whole lot of caching checks, validities, etc.).

@david-german-tri:

No, but that's not part of the API that System provides; it's an API that System uses.

That API is also users' API right? I mean, users will write their own systems (plants) that they'd like to simulate, analyze, control, do V&V on them, etc.

[david-german-tri]

The Calc convention in general is fine with me. I am not convinced that we should NVI it, though; individual system authors can judge whether it's necessary to factor CalcFoo out of EvalFoo or not.

That API is also users' API right? I mean, users will write their own systems (plants) that they'd like to simulate, analyze, control, do V&V on them, etc.

Yes, of course. I'm saying that the costs to providing it later are small: a few Systems get written with an extra copy, and can easily be optimized later when and if anyone cares.

[liangfok]

What does NVI stand for? No obvious candidates here: http://www.acronymfinder.com/NVI.html.

[jwnimmer-tri]

Google for NVI C++ gets good hits. Here's one; the Sutter articles are also good:
https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Non-Virtual_Interface

[liangfok]

Thanks!

[sherm1]

The Calc convention in general is fine with me. I am not convinced that we should NVI it, though; individual system authors can judge whether it's necessary to factor CalcFoo out of EvalFoo or not.

For the few "Eval" methods that are part of the System API I think there are good arguments for making _only_ the Calc methods virtual. The Eval methods can be non-virtual boiler plate for output ports, derivatives, updates, and a few other things where we know the value source location. @amcastro-tri made the best argument, which is that there is no reason normally to involve System-writing users in the cache minutiae -- they should think about computation; we can decide for ourselves when to perform that computation.

The second reason to do this is technical. It is important for speed that Eval methods are inlined (I have measured this in Simbody), because (a) they are called in simulation inner loop computations, and (b) their most common execution path is: (1) check flag; see computation is valid, (2) return reference. Actual function call overhead costs substantially more than the execution time. If the routine itself looks as I sketched it above, the compiler inlines it because the total size of the method is small (only a single function call is added). If the code to actually perform the computation when it is needed is written in the body of the Eval method, the total size of the method almost always exceeds the compiler's inlining threshold.

Using a separate method for computation is a best practice here for reasons we don't want to have to discuss with every user. Typically the best way to handle that is to model it ourselves in all our code and examples so that people will do it the right way by default.

[david-german-tri]

OK, I'm convinced. Partly by that argument, partly by #3173. I think we should fix this in one shot with #2890, and after #3164 lands.

[david-german-tri]

Also, whatever mechanism we use to fix this should also apply to state derivatives, and other things that get Evaled. See discussion on #3218.

[david-german-tri]

That, in turn, will be easier after all vectors are unified, so I think this whole issue is now blocked on #3208.

[tri-ltyyu]

Still relevant

[amcastro-tri]

@sherm1, I believe this is resolved now! re-open if I'm wrong.

[jwnimmer-tri]

No, it is not resolved. Sherm and I (and Lesley) specifically discussed this on Friday.

[sherm1]

@amcastro-tri this refers to being able to declare an output port that doesn't have its own cache entry but rather just exposes some other Context object like state variables or someone else's cache entry. We don't have that option yet.