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

Declare all internal states as drake State or CacheEntry #311

Open
Brian-Acosta opened this issue Jul 19, 2022 · 1 comment
Open

Declare all internal states as drake State or CacheEntry #311

Brian-Acosta opened this issue Jul 19, 2022 · 1 comment
Assignees

Comments

@Brian-Acosta
Copy link
Contributor

So far we have been relatively laissez faire when it comes to storing controller state (i.e. previous OSC solutions, tracking data, etc.) as member variables instead of in some way that would expose them to the relevant context. This seems to be causing weird bugs with efforts to gymify our controllers, as some internal state variables are not being reset with the rest of the context. We should consider refactoring as these issues come up and being more strict in subsequent code review to avoid these kinds of design patterns.

@Brian-Acosta
Copy link
Contributor Author

Something I learned trying to write a controller complying with these rules: to store something as an AbstractState or CacheEntry, it must be able to be stored as an AbstractValue , which requires that it be a copyable object or a cloneable pointer.

Many drake objects (like MathematicalProgram ) have explicitly deleted copy constructors and copy assignment operators, but can be stored as an abstract value using drake's copyable_unique_ptr wrapper. E.g. drake::copyable_unique_ptr<MathematicalProgram> prog(std::make_unique<MathematicalProgram>()).

Additionally, if you write a container class for your optimization which holds the MP, the decision variables, and the constraints, you can use drake::copyable_unique_ptr<MathematicalProgram> to ensure that the default copy constructor and copy assignment operators of your container class aren't implicitly deleted. This way you avoid having to write your own copy assignment operator, which can get tricky.

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

No branches or pull requests

3 participants