Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add warmstart implementation for warmstart from reference points #111
base: topic/humble-devel/refactor
Are you sure you want to change the base?
Add warmstart implementation for warmstart from reference points #111
Changes from all commits
d6a3a14
4be5ded
aa16841
13536bd
8d00f8e
abf790c
0bafb70
e9a6f4d
9a1be8f
a51d9bf
a590c7f
42f6b37
ec61b1f
ad2cfc2
27d4ed1
63853bb
3b221ad
4f412ea
bdb1c21
31a9032
c6a3e16
0b65d7b
72ff312
c07b713
5587b58
54e419d
6c2e874
fb240f8
6c8c537
fb10156
f4e88ec
06032a4
dd328d4
6d6277d
23f0da7
984f988
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not 100% sure if we should document private attributes. Maybe an "inline" comment would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Methods should have documentation coming from their own docstring. There is no need to document them at the global level
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those arguments are private, so I don't see a point of documenting them as a part of public API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned before. This documentation will be inherited from the base class and method docstrings itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation for those arguments might be placed in here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we assume that warmstart in general doesn't need pinocchio? This function generally breaks compatibility with the abstract class so is there a good explanation why the base class dosn't have
setup
function?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't
self._rmodel.nq + self._rmodel.nv
be simply stored in a variable likestate_size
? Maybe evenself._state_size
, that would be updated insetup()
function