Per f2f with @david-german-tri and @edrumwri, and a suggestion by @jwnimmer-tri, we would like to have a better name for the ubiquitous and extremely useful contiguous vector type currently called BasicVector, which is very easy to confuse with its base class, VectorBase. We could name it RawVector in which case the hierarchy would be
VectorBase
RawVector // Owns an actual, contiguous Eigen Vector.
Subvector // Refers to a subrange of a VectorBase.
Supervector // A concatenation of VectorBases.
If you don't like that name, feel free to propose another one.
(See #3208 and related PRs for discussion and much hard work by David.)
Another idea is ProtoVector.
Consider renaming BasicVector, instead of VectorBase.
Consider renaming BasicVector, instead of VectorBase
Good idea. I revised the title and description of this issue accordingly, PTAL. I'm nominating RawVector as the replacement name for the guaranteed-contiguous vector formerly known as BasicVector. (This is the only VectorBase subclass from which you can obtain an underlying Eigen VectorX<T>.) Any objections or alternate proposals? I also considered ContiguousVector (seems too long?) and EigenVector (already in use in Drake for something else and annoyingly similar to an unrelated linear algebra concept).
I'm OK with BasicVector -> RawVector. I would prefer BasicVector -> ContiguousVector, or VectorBase -> AbstractVector, but it's your bikeshed to paint.
Naming is hard. I think RawVector would be fine. The only other possibly-better ideas I came up with are DirectVector or SimpleVector or just Vector.
The more I think about it, the more merit I see in ContiguousVector. Raw is kind of ambiguous.
I would be happy with ContiguousVector. It is a bit long but certainly accurate and clear.
I lost enthusiasm for ContiguousVector as a rename for BasicVector after I realized that SubVectors are often contiguous also. And now I've been staring at VectorBase/BasicVector for so long I'm not adequately confused by the name similarity any more. So I'm tempted to close this. Would someone with a fresher point of view please say whether we should still change one of the names? RawVector would avoid the name similarity and mean roughly as little as Basic does.
For those of us who have been working with BasicVector / VectorBase long
enough, we're no longer confused, but the current naming sucks- a user
shouldn't need to use both types over and over to recall the distinction.
To provide a problem without a solution: I don't like RawVector. It's not a
piece of meat! If you're going down that road, how about AtomicVector?
Evan Drumwright
Research Scientist
http://positronicslab.github.io
Toyota Research Institute
Palo Alto, CA
On Wed, Feb 1, 2017 at 4:54 PM, Michael Sherman notifications@github.com
wrote:
I lost enthusiasm for ContiguousVector as a rename for BasicVector after
I realized that SubVectors are often contiguous also. And now I've been
staring at VectorBase/BasicVector for so long I'm not adequately confused
by the name similarity any more. So I'm tempted to close this. Would
someone with a fresher point of view please say whether we should still
change one of the names? RawVector would avoid the name similarity and
mean roughly as little as Basic does.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RobotLocomotion/drake/issues/3614#issuecomment-276832969,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACHwzxChCmqe2EHIk2WQ1jbBNxWq2Mfrks5rYSlGgaJpZM4KJcai
.
I think we should avoid Atomic because of the specific meaning in computing. Contiguous remains the best suggestion on this thread, IMO; it doesn't bother me that subvectors are often contiguous.
Same here- doesn't bother me that subvectors are often contiguous.
Evan Drumwright
Research Scientist
http://positronicslab.github.io
Toyota Research Institute
Palo Alto, CA
On Thu, Feb 2, 2017 at 7:18 AM, David German notifications@github.com
wrote:
I think we should avoid Atomic because of the specific meaning in
computing. ContiguousVector remains the best suggestion on this thread,
IMO; it doesn't bother me that subvectors are often contiguous.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RobotLocomotion/drake/issues/3614#issuecomment-276985794,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACHwzz-J7OuMWQTP1WzpxFnzJX3g1eogks5rYfPMgaJpZM4KJcai
.
+1 for ContiguousVector. I remember my confusion at the beginning when they were just introduced. The fact that "Basic" and "Base" sound so similar would make me think twice about which was which.
If we rename BasicVector to ContiguousVector, would it be a good idea to rename Supervector to something like CompoundVector or something that reflects the fact that it is not contiguous but a set or composition of contiguous pieces?
That would give us:
ContiguousVector
CompoundVector
Subvector
which does seem like an improvement. Does Subvector need a new name also?
I'm not sure I like the Supervector -> CompoundVector change. We want to rename BasicVector because it's too easily confused with VectorBase. What's the motivation for renaming Supervector, or Subvector for that matter?
User code should rarely or never refer to a Supervector or Subvector. They are implementation details for Diagram continuous state, and present to users as a VectorBase.
What's the motivation for renaming Supervector, or Subvector
Feature creep!
Good point, David. Back to:
ContiguousVector
Supervector
Subvector
Is that the winner?
I am not sure I'd understand "Supervector" without further context. Subvector is clear (although Eigen calls it VectorBlock). I admit that I dislike the fact that the V in vector is caps sometimes.. it means I'll almost certainly type it incorrectly for a while.
Russ makes good points with regard to immediately understanding the
semantics of supervector and with respect to consistency of capitalization.
I think these are second order issues compared to renaming BasicVector.
Evan Drumwright
Research Scientist
http://positronicslab.github.io
Toyota Research Institute
Palo Alto, CA
On Sat, Feb 4, 2017 at 4:41 AM, Russ Tedrake notifications@github.com
wrote:
I am not sure I'd understand "Supervector" without further context.
Subvector is clear (although Eigen calls it VectorBlock). I admit that I
dislike the fact that the V in vector is caps sometimes.. it means I'll
almost certainly type it incorrectly for a while.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RobotLocomotion/drake/issues/3614#issuecomment-277443538,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACHwz31NUy5tzxAdygzpD_msH0rrBT-vks5rZHHigaJpZM4KJcai
.
I agree with Evan. Just to reiterate, I expect that only users who are aggressively rummaging under the sofa cushions will ever need to type Supervector or Subvector. For that matter, most Drake core devs won't encounter them.
Right! I have used VectorBase and BasicVector extensively, even adding some
functionality to them, and have yet to touch Supervector or Subvector.
Evan Drumwright
Research Scientist
http://positronicslab.github.io
Toyota Research Institute
Palo Alto, CA
On Mon, Feb 6, 2017 at 7:46 AM, David German notifications@github.com
wrote:
I agree with Evan. Just to reiterate, I expect that only users who are
aggressively rummaging under the sofa cushions will ever need to type
Supervector or Subvector. For that matter, most Drake core devs won't
encounter them.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RobotLocomotion/drake/issues/3614#issuecomment-277721966,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACHwz3zN2Tk4lZNK7enE_OUimL9s94Fvks5rZ0BogaJpZM4KJcai
.
Per discussion in #5138 and elsewhere, deriving from cloneable, concrete BasicVector (ContiguousVector) is problematic since there is no way to enforce proper reimplementation of DoClone() in the derived classes. Per a Slack discussion I've lost track of, this could be fixed by introducing @soonho-tri's idea of an abstract class containing (and properly cloning) a contiguous Eigen vector but leaving DoClone() unimplemented:
VectorBase
Supervector
Subvector
ContiguousVectorBase // abstract
ContiguousVector // final (replaces BasicVector)
Then classes like AccelerometerOutput would derive from ContiguousVectorBase and be final.
The longer we wait to rename one of the most essential classes in the tree, the more annoying it's going to be as its used in more APIs and more downstream projects.
The last comment in this thread was over a year ago (and made the point that we should do this asap if we're going to do it at all). I'm going to close this as "nice idea but we aren't going to do it".
Most helpful comment
Good point, David. Back to:
Is that the winner?