Parameters need an upgrade to first-class Diagram elements

[RussTedrake]

See discussion below. Parameters should be treated consistently with our state variable treatment.

[sherm1]

Agreed. We've been very inconsistent about this. I will fix this shortly with some other cleanups. In the System framework at least, I'd like to get to a consistent API where function returns are references unless they can actually return nullptr.

[RussTedrake]

i'll keep calling them out as I come across them.
Comically, it looks like Context::get_mutable_abstract_parameter returns a (mutable) reference, instead of a pointer.

[RussTedrake]

also: num_numeric_parameters should be get_num_numeric_parameters for consistency with the other methods. Same for num_abstract_parameters.

[sherm1]

num_numeric_parameters should be get_num_numeric_parameters ...

I've been moving the other direction lately. The style guide allows dropping "get_" for simple getters like this, and now that I've tried it I can see why. The code reads a little better and takes up less space. Also for counts like this the STL precedent is size() which is nicer than get_size(). Our code base has a somewhat random mix at the moment -- we should be consistent either way. I would vote now for num_things(), get_thing(i), get_mutable_thing(i) for indexed objects, with the latter returning references rather than pointers.

[RussTedrake]

Right, my ask is for consistency... i don't have a strong preference on the exact form. Since you've raised it more generally, one other notable case, which is inconsistent with your proposal, is the generated named vectors -- these have accessors thing() and setters set_thing() (eg theta() and set_theta() in pendulum). I guess I'm fond of that version, but admit it might not work everywhere.

[sherm1]

accessors thing() and setters set_thing() ...

I also like that pairing for and would like to keep it. It doesn't really work when there is also a possibility to return a mutable thing, assuming we want to make it clear that we're doing that (and I think we do since grabbing a mutable object may cause cache invalidation). We could consider just mutable_thing() in that case though and leave off the get_. Then in the indexed case we'd have num_things() thing(i) mutable_thing(i) and possibly set_thing(i, value).

[RussTedrake]

Another inconsistency with parameters... there are no methods GetSubsystemParameters nor GetMutableSubsystemParameters in Diagram. My PR #7145 works around this for the particular case of SetDefaultParameters, but the API should mirror the API for states. (Note that this is because Parameters were originally not defined at the Diagram level... that was a patch that came later and they continue to exist as a second class citizen).

[sherm1]

The original consistency issue (numeric parameter methods inconsistent with abstract parameter methods) was fixed in #7312, and similar issues fixed in #7337. But the discussion above noted some other problems with Parameters. I've updated the title and will leave this open.

[RussTedrake]

I will add to this list, rather than open another issue. Here are some more inconsistencies:

• We have get_numeric_parameter(index), but don't offer the get_numeric_parameter() for the case where there is exactly one parameter. Same for get_abstract_parameter().
• We have DeclareNumericParameter(BasicVector) but don't offer DeclareNumericParameter(Eigen::VectorXd). This is again inconsistent with the way we declare ports and state vectors.

These were motivated by writing a systems framework tutorial (will PR shortly), which also makes me think we should renaming "numeric" parameters to "vector" parameters. That would be more consistent with input/output ports, etc.