Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abstraction for TraversalState #115

Closed
robfitzgerald opened this issue Feb 7, 2024 · 1 comment · Fixed by #136
Closed

Abstraction for TraversalState #115

robfitzgerald opened this issue Feb 7, 2024 · 1 comment · Fixed by #136
Assignees
Milestone

Comments

@robfitzgerald
Copy link
Collaborator

robfitzgerald commented Feb 7, 2024

A TraversalModel is currently responsible for knowing what the meaning of a TraversalState and providing methods for interacting with the state. many methods may create a new state vector in order to do update work or otherwise. in a few ways, there is actually a need to share the interface for a TraversalState between different modules:

  • TraversalModel: module that does state updates due to link traversals
    • EnergyTraversalModel has specific state for the Vehicle model
  • AccessModel: forthcoming module that does state updates dues to link access rules
  • FrontierModel: module that uses state to determine if links are valid for a traversal

In order to share the knowledge of the state between all of the above, we could create an abstraction that provides methods for interacting with a state vector and share it between the above modules. a traversal state object would still be a Vec but the TraversalStateModel would track name to index information, would be the factory for cloning/allocating a new mutable vector, and perhaps even hold unit information.

this solves a performance issue by allowing the downstream models to operate on a mutable state vector created by the TraversalStateModel and share the same behaviors for interacting with that vector during a search iteration. this way, a new vector is created once per iteration. it also allows us to automate vector sizing for any TraversalState to the known state representation size using Vec::with_capacity(n) to ensure all state vectors are as small as possible.

@robfitzgerald robfitzgerald added this to the Performance milestone Feb 7, 2024
@nreinicke
Copy link
Collaborator

This is great and I think having this abstraction will really help us with performance and general code readability.

Something else on this topic that I noticed while working through the memory profiling is the fact that a vector has some overhead (24 bytes) for holding onto these state variables. So in the case of our energy traversal model with a ICE vehicle we have a total of 3 f64 state variables which at a minimum would take up 24 bytes but end up taking 48 bytes in our current Vec representation. Another nice thing about using the TraversalStateModel abstraction is that each traversal state implementation could store its state variables as a fixed size array. This would be a smaller memory footprint and would be on the stack which would make for quicker access and copies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants