-
Notifications
You must be signed in to change notification settings - Fork 1
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
Create Home Position Reward #33
Conversation
Pull Request Test Coverage Report for Build 427141405
💛 - Coveralls |
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.
You have the same code in one of the other reviews so those comments are applicable. I can approve this PR and we can resolve any concerning issues in the PR after this?
Since this is one of the earlier PRs, do you want me to merge this one first? idk if you left most of your comments in the other PRs or what would make this easiest for ya... |
I am okay with you merging this in because I looked at the other PR before this so all my relevant comments are there |
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.
These changes are present in another PR. All relevant comments are present in that PR. Giving this PR approval based on this
Closes #24.
Basically as mentioned before, this gives the maximal reward (~1) for the robot reaching the home position (quadrupedal standing). Big thing to note here is that the reward is dependent on both the height and the orientation, as shown in the line below:
gym_solo/gym_solo/core/rewards.py
Line 146 in 0c96a33
However, notice that these are being weighted and summed together. This functionality is also built into our
rewards_factory
, but I just thought it was cleaner to implement it this way as the quadrupedal standing max height will be different from the bipedal standing one. However, if the relative weights between the two rewards needs to be optimized, then they will probably have to be split up so that W&B and tune it for us.Note that #32 should probably be merged first cause that renames the
test_rewards.py
file, which this PR modifies. This PR is branched off of the previous one, so if #32 gets merged, this shouldn't have any problems.