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

[Feature] Add env wrapper for Unity MLAgents #2469

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

kurtamohler
Copy link
Collaborator

@kurtamohler kurtamohler commented Oct 7, 2024

Description

Adds UnityMLAgentsWrapper and UnityMLAgentsEnv environment wrapper classes to control MLAgents environments from TorchRL.

Motivation and Context

close #1110

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

Types of changes

What types of changes does your code introduce? Remove all that do not apply:

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

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • I have read the CONTRIBUTION guide (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.

@kurtamohler kurtamohler added the Environments Adds or modifies an environment wrapper label Oct 7, 2024
@kurtamohler kurtamohler requested a review from vmoens October 7, 2024 20:08
Copy link

pytorch-bot bot commented Oct 7, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/2469

Note: Links to docs will display an error until the docs builds have been completed.

❌ 15 New Failures, 6 Unrelated Failures

As of commit dd93644 with merge base f411f93 (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 7, 2024
@kurtamohler kurtamohler changed the title Add env wrapper for Unity MLAgents [Feature] Add env wrapper for Unity MLAgents Oct 7, 2024
Copy link
Contributor

@vmoens vmoens left a comment

Choose a reason for hiding this comment

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

That looks amazing!
Can you share a bit about the current limitations (eg, are the specs expected to be fixed?) and some context about the tests - I see a couple of mocks and pytest marks


@pytest.mark.unity_editor
def test_with_editor(self):
print("Please press play in the Unity editor") # noqa: T201
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When UnityMLAgentsEnv is not given a path to a file or the name of a registered env, it waits for the user to press the play button in the unity editor or run a mlagents-compatible binary. Since we can't easily do that in CI, I figured we can just skip the test by default. To run the test, the cmd line arg --unity_editor has to be given. We could remove this test, but I found it useful while working on this PR to have a test that connects to whichever environment I have open in the unity editor

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha!

@kurtamohler kurtamohler force-pushed the unity-mlagents-0 branch 2 times, most recently from 1fd36a0 to 0a84b47 Compare October 8, 2024 17:12
@kurtamohler
Copy link
Collaborator Author

kurtamohler commented Oct 8, 2024

Can you share a bit about the current limitations (eg, are the specs expected to be fixed?) and some context about the tests - I see a couple of mocks and pytest marks

Current limitations are:

  • The specs are expected to be fixed throughout the run, so no new agents, behaviors, or observations can be added during the run.
  • All agents are expected to request a decision in the first step so that they all get included when the specs are built.
  • Because of the previous two points, the "Basic" and "DungeonEscape" environments from the example environments included with MLAgents do not work with our wrapper classes. But the ~15 other example envs do work.
  • I haven't added the group_map argument to the wrappers.

For the tests:

  • MLAgents comes with a few built-in environment binaries that can be automatically downloaded and run, so we have a test that uses those. For now, I only enabled testing two of those environments because each download is 180 MB. But thankfully they just get downloaded to /tmp, which is cleared on shutdown. We could add more of them to the list.
  • The mock tests are modeled after the tests in the mlagents repo. I figured if that's how they test their environment class, we might as well do it that way too. Also, the registered envs are still an "experimental" feature, so I thought maybe we shouldn't only rely on those.

@vmoens vmoens merged commit 205b83d into pytorch:main Oct 10, 2024
52 of 69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Environments Adds or modifies an environment wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Unity MLAgents Support
3 participants