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.

Sign up for GitHub

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

Jump to bottom

Fixed parameter order for compute_reward #725

Merged
merged 6 commits into from
Mar 21, 2020
Merged

Fixed parameter order for compute_reward #725

merged 6 commits into from
Mar 21, 2020

Conversation

johannes-dornheim
Copy link

@johannes-dornheim johannes-dornheim commented Mar 6, 2020 •
edited
Loading

the openai-gym signature compute_reward(self, achieved_goal, desired_goal, info) was not matched, which leads to errors when the reward is not symmetric regarding desired_goal and achieved_goal.

Description

Motivation and Context

  • I have raised an issue to propose this change (required for new features and bug fixes)

closes #728

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)

Checklist:

  • I've read the CONTRIBUTION guide (required)
  • I have updated the changelog accordingly (required).
  • My change requires a change to the documentation.
  • I have updated the tests accordingly (required for a bug fix or a new feature).
  • I have updated the documentation accordingly.
  • I have ensured pytest and pytype both pass (by running make pytest and make type).

@johannes-dornheim
fixed parameter order for compute_reward
7b0a6ce 
the openai-gym signature compute_reward(self, achieved_goal, desired_goal, info) was not matched, which leads to errors when the reward is not symmetric regarding desired_goal and achieved_goal.
@johannes-dornheim
Copy link
Author

the openai-gym signature compute_reward(self, achieved_goal, desired_goal, info) was not matched, which leads to errors when the reward is not symmetric regarding desired_goal and achieved_goal.

@araffin
Copy link
Collaborator

araffin commented Mar 6, 2020

Hello,
Thanks for the PR, but please actually check the boxes if you did what is written...

@johannes-dornheim
Copy link
Author

johannes-dornheim commented Mar 6, 2020 •
edited
Loading

Hi,

Sry that I forgot to create the related issue earlier. Now I've done so.

@araffin
Copy link
Collaborator

araffin commented Mar 7, 2020

LGTM, if you read the contribution guide, then you know what is missing for it to be merged...

@araffin araffin changed the title fixed parameter order for compute_reward Fixed parameter order for compute_reward Mar 7, 2020
araffin
araffin approved these changes Mar 21, 2020
View reviewed changes
Copy link
Collaborator

@araffin araffin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM =)

@araffin araffin merged commit 49b1ba6 into hill-a:master Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@araffin araffin araffin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Signature of GoalEnv.compute_reward not matched in HER replay-buffer code
2 participants
@johannes-dornheim @araffin