Drake: Combine the *StateVector hierarchy and the VectorBase hierarchy.

Created on 22 Aug 2016  路  13Comments  路  Source: RobotLocomotion/drake

LeafSystemSateVector should be the last class in the hierarchy of state vectors. All the additional functionality from BasicStateVector can safely be moved up into LeafSystemStateVector.

This will allow us to access for instance to safely access the contiguous in memory Eigen representation for LeafSystems without unsafe dynamic_cast's.

high dynamics

Most helpful comment

Could you provide as you did above for #3296 how the hierarchy will look after that sequence of PR's is complete?

Sure, it's what @sherm1 said above:

  BasicVector // contiguous, can have `get_[mutable_]eigen()` (or whatever)
  Supervector // assembles multiple VectorBases (so might be non-contiguous)
  Subvector   // selects a subrange of a VectorBase (so might be non-contiguous)

All 13 comments

cc'in @david-german-tri and @sherm1

@amcastro-tri, see discussion in #3151. I would like to see VectorInterface extended to support "chunked" non-contiguous vectors and subvectors thereof. Then we can use that one abstraction exclusively for output ports, states, continuous state derivatives, etc. allowing us to get rid of the separate StateVector abstraction. That would also make it very straightforward to pump state variables (or some of them) out an output port.

I studied the code and it seems that the change we'd be suggesting would be:

  1. VectorInterface would replace StateVector.
  2. BasicVector would replace BasicStateVector and LeafStateVector (effectively merging them as suggested in this issue).
  3. Make StateSupervector inherit the VectorInterface and rename it to something like SlicedVector.

After those changes we would have:

  • BasicVector implementing VectorInterface for contiguous in memory vectors and SlicedVector for non-contiguous in memory vectors.
  • We would get rid of all the State classes including: StateVector, StateSupervector, BasicStateVector and LeafStateVector greatly reducing the number of abstractions and hopefully simplifying our framework.

@david-german-tri and @sherm1 what do you think? should I execute?

That (much larger!) set of changes would mean we can't provide get_value or get_mutable_value on VectorInterface anymore, because there wouldn't necessarily be an underlying Eigen type. That might actually be fine, though. The lack of those methods hasn't been a pain point for StateVector.

Nit: I think we should say Subvector and Supervector, not "SlicedVector" which sounds like it could mean either one.

Reassigned to me based on off-thread discussion.

I think we should say Subvector and Supervector, not "SlicedVector" which sounds like it could mean either one.

I was just going to say that -- :+1:. And those would both derive from VectorInterface, right? Would the hierarchy then be:

VectorInterface
  BasicVector // a single chunk, can have `get_[mutable_]eigen()` (or whatever)
  SuperVector // assembles multiple VectorInterfaces? 
  SubVector   // selects a subrange of a VectorInterface (so might be non-contiguous)

?

Yes.

Also, having gotten partway into this, it is clearly far too messy to do in one PR. There will be several pieces.

Yes.

Awesome! I love that we can focus all our vector abstraction & performance attention on that one class family.

3296 is the next phase of this migration. After that PR, we will have a single hierarchy:

  StateSupervector
  StateSubvector
  VectorBase
    BasicVector
      BasicStateVector
        BasicStateAndOutputVector

The next PR will delete BasicStateVector and BasicStateAndOutputVector. The PR after that will make Supervector and Subvector children of VectorBase. The final PR will fold StateVector into VectorBase.

The next PR will delete BasicStateVector and BasicStateAndOutputVector. The PR after that will make Supervector and Subvector children of VectorBase. The final PR will fold StateVector into VectorBase.

Just to be clear. Could you provide as you did above for #3296 how the hierarchy will look after that sequence of PR's is complete? Thanks!

Could you provide as you did above for #3296 how the hierarchy will look after that sequence of PR's is complete?

Sure, it's what @sherm1 said above:

  BasicVector // contiguous, can have `get_[mutable_]eigen()` (or whatever)
  Supervector // assembles multiple VectorBases (so might be non-contiguous)
  Subvector   // selects a subrange of a VectorBase (so might be non-contiguous)

3393 didn't get linked to this PR, and so I forgot I had done it. This issue is actually complete! 馃帀

Was this page helpful?
0 / 5 - 0 ratings

Related issues

EricCousineau-TRI picture EricCousineau-TRI  路  3Comments

SeanCurtis-TRI picture SeanCurtis-TRI  路  4Comments

jwnimmer-tri picture jwnimmer-tri  路  4Comments

mntan3 picture mntan3  路  4Comments

jamiesnape picture jamiesnape  路  5Comments