Skip to content

Improvements to Learning-Environment-Create-New.md #3773

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

Merged
merged 4 commits into from
Apr 17, 2020

Conversation

mmattar
Copy link

@mmattar mmattar commented Apr 14, 2020

  • Changed the ordered list to use "1."
  • Trimmed down text
  • Removed reference to materials as those are in the Example Envs project

Proposed change(s)

Describe the changes made in this PR.

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

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

- Changed the ordered list to use "1."
- Trimmed down text
- Removed reference to materials as those are in the Example Envs project
@mmattar mmattar requested a review from chriselion April 14, 2020 23:29

### Initialization and Resetting the Agent

When the Agent reaches its target, its episode ends and the `OnEpisodeBegin()`
method moves the target to a random location. In addition, if the Agent rolls
When the Agent (Sphere) reaches its target (Cube), its episode ends and the `OnEpisodeBegin()`
Copy link
Contributor

Choose a reason for hiding this comment

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

This paragraph doesn't actually talk about "Initialization" even though it's in the section title. Maybe mention that Initialization is called at the start of simulation too?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch.

To solve the task of moving towards the target, the Agent (Sphere) needs to be able to
move in the `x` and `z` directions. As such, we will provide 2 actions to the agent.
The first determines the force applied along the x-axis; the second
determines the force applied along the z-axis. (If we allowed the Agent to move in three dimensions, then we would need a third action.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: line length

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

actionsOut[0] = Input.GetAxis("Horizontal");
actionsOut[1] = Input.GetAxis("Vertical");
}
public override float[] Heuristic()
Copy link
Contributor

Choose a reason for hiding this comment

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

Old code was correct, see #3765

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

@@ -473,7 +420,7 @@ localPosition rather than position, so that our position reference is in referen
to the prefab TrainingArea's location, and not global coordinates.

1. Replace all references of `this.transform.position` in RollerAgent.cs with `this.transform.localPosition`.
2. Replace all references of `Target.position` in RollerAgent.cs with `Target.localPosition`.
1. Replace all references of `Target.position` in RollerAgent.cs with `Target.localPosition`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just start with these in the example, instead of changing them later? There's been some confusion before because the examples conflict with the best practices that we call out elsewhere.

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense.

@mmattar mmattar requested a review from chriselion April 17, 2020 02:49
In many of the [example environments](Learning-Environment-Examples.md), many
copies of the training area are instantiated in the scene. This generally speeds
up training, allowing the environment to gather many experiences in parallel.
This can be achieved simply by instantiating many Agents which share the
Copy link
Contributor

Choose a reason for hiding this comment

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

which share the Behavior Parameters

This isn't really accurate; they should have identical Behavior Parameters but they're not actually "shared" .

Copy link
Author

Choose a reason for hiding this comment

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

Good catch - fixed it.

Marwan Mattar added 2 commits April 16, 2020 21:07
removed unnecessary configs
updated the agent image
@mmattar mmattar merged commit 4616a50 into master Apr 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the develop-mm-docs-learning-envs branch April 17, 2020 04:12
@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