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

[change] Remove concatenate in discrete action probabilities to improve inference performance #3598

Merged
merged 4 commits into from
Mar 11, 2020

Conversation

ervteng
Copy link
Contributor

@ervteng ervteng commented Mar 9, 2020

Proposed change(s)

Discrete action models in ML-Agents produces outputs for action probabilities, one per branch, concatenates them, then breaks them out again to get the actions themselves. see:

Screenshot 2020-03-09 at 22 30 13_annotated

This causes redundant cache-copying behavior in Barracuda and may slow inference performance quite a bit.

This PR changes the network architecture to below to avoid this behavior:

image (1)

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Breaking change
  • Documentation update
  • Other (please describe)

Checklist

  • Added tests that prove my fix is effective or that my feature works
  • Updated the changelog (if applicable)
  • Updated the documentation (if applicable)
  • Updated the migration guide (if applicable)

Other comments

@ervteng ervteng requested review from mantasp and chriselion March 9, 2020 22:11
@ervteng ervteng requested a review from andrewcoh March 11, 2020 00:11
@chriselion
Copy link
Contributor

Maybe add something to the changelog?

@ervteng ervteng merged commit 6dbba73 into master Mar 11, 2020
@delete-merged-branch delete-merged-branch bot deleted the develop-parallelizeinference branch March 11, 2020 18:01
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 15, 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.

2 participants