Skip to content

Conversation

@ervteng
Copy link
Contributor

@ervteng ervteng commented Aug 14, 2019

Had to make additional changes to reward signals and rl_trainer to abstract to off-policy.

  • Adds evaluate_batch to reward signals. Evaluates on minibatch rather than on BrainInfo.
  • Changes the way reward signal results are reported in rl_trainer so that we get the pure, unprocessed environment reward separate from the reward signals.
  • Moves end_episode to rl_trainer

@ervteng ervteng requested a review from harperj August 14, 2019 00:05
@ervteng ervteng changed the base branch from master to develop August 14, 2019 00:05
Copy link
Contributor

@harperj harperj left a comment

Choose a reason for hiding this comment

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

A couple of questions and minor suggestions, but mostly LGTM.

@harperj
Copy link
Contributor

harperj commented Aug 15, 2019

Latest changes LGTM. Can you briefly summarize how you tested this PR?

@ervteng
Copy link
Contributor Author

ervteng commented Aug 15, 2019

Latest changes LGTM. Can you briefly summarize how you tested this PR?

Tested GAIL with Crawler and Curiosity + GAIL with Pyramids. Tested GAIL with GridWorld for visual obs.

Copy link
Contributor

@harperj harperj left a comment

Choose a reason for hiding this comment

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

Sounds good, approved!

@ervteng ervteng merged commit 27cd6dd into develop Aug 15, 2019
@ervteng ervteng deleted the develop-rewardsignalsforsac branch August 15, 2019 22:29
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants