Drake: use variable names instead of long text to access elements of the context

[RussTedrake]

Following the discussion with @david-german-tri and @sherm1 in #3735, and to go along with #3751, I'd like to discuss whether we should make the following substitutions. @sherm1 's comment:

In my fantasy world, I picture this set of methods, each returning an indexable vector:

get_x()  // returns continuous and difference state variables together
get_xc() // where x={xc,xd}
get_xd()
get_q()  // where xc={q,v,z}
get_v()
get_z()

It seems awkward to invent long names for all of these when the math is so concise. But I'm OK with the long names 😢.

I was on the fence yesterday, but having slept on it, I'm now in favor of the change.

• the x(2) would arguably lead to much more readable code for simple systems
• @sherm1's point is that "continuous_state" is still an incomplete and potentially confusing description when what we mean is "state variables that evolve continuously in time". Using xc instead also requires somebody to read our documentation once to understand, and is just as memorable if it is standardized throughout the code. (it's already used exclusively in matlab).
• the bigger reason for me, which occurred to me last night, is that I think we will often document our systems using systems of equations... including doxygen's mathjax support and also my textbook documentation, and those equations will use the x_c x_d syntax. It's much better if they match!

I'd actually go a step further, and drop the get_ on the accessor methods. We also discussed having syntactic sugar, e.g. x(index i) which returns the specific element. Perhaps set_x(index i, T value) for the setters. i would love to avoid the mutable everywhere to return a const ref when the context is const and a non-const-ref when it's not, but am not sure i know how.

• @jwnimmer-tri , @siyuanfeng-tri , and @tkoolen who might have strong opinions and different perspectives. (all comments welcome!)

[RussTedrake]

a counter-argument (against myself) is that the most readable code for simple systems might prefer to make local variable references like this:

auto x = context.get_continuous_state_vector();
auto xdot = output.get_mutable_vector();

xdot(0) = x(0) + 3*x(2) …

which is cleaner to read because it doesn’t have the context.x(0) sprinkled everywhere.

[david-german-tri]

It's already pretty common in our code to assign the xc pointer to a local variable, especially when the state vector has semantics. This is a healthy pattern! Example: https://github.com/RobotLocomotion/drake/blob/master/drake/systems/plants/spring_mass_system/spring_mass_system.cc#L155

As a reader of code, even if continuous_state isn't a totally complete description, it gives me a lot more to go on than xc does. I remember as TRI was ramping up on Drake in February and March, cryptic, alphabetic variable names were a huge pain point. I'd be sad to reintroduce them; I finally have the background knowledge to cope, but the next person like me won't.

Separately, I have no objection to dropping the get_ prefixes, as long as we're consistent about it.

[sherm1]

Despite my whining quoted above, I am OK with the long names and like the pattern of using them to populate mathematically-named local references. With the long names I think we should keep the initial get_ also. And we definitely need to make it obvious when we ask for mutable access to any state because in our system that has the serious and potentially very expensive consequence of invalidating downstream dependents. get_mutable_ does get the message across though possibly GetMutable... would scream it more convincingly.

[RussTedrake]

I agree. For my purposes, embracing the local naming solves the problem, and nullifies any requests to remove the get_ . I still like all of the suggestions in #3751. Will close this one.