Skip to content

Split mlagents into two packages #1812

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 28 commits into from
Apr 1, 2019
Merged

Split mlagents into two packages #1812

merged 28 commits into from
Apr 1, 2019

Conversation

awjuliani
Copy link
Contributor

This PR splits the mlagents package into to packages. One is mlagents_envs which contains only the python api, and is nice and lightweight. The second is mlagents which contains the PPO/BC trainer and depends on mlagents_envs.

vincentpierre and others added 10 commits January 11, 2019 15:19
* Add timeout wait param

* Remove unnecessary function
…ackage

# Conflicts:
#	UnitySDK/Assets/ML-Agents/Scripts/CommunicatorObjects/UnityToExternalGrpc.cs
#	gym-unity/tests/test_gym.py
#	ml-agents-envs/mlagents_envs/environment.py
#	ml-agents/mlagents/trainers/trainer.py
#	ml-agents/mlagents/trainers/trainer_controller.py
#	ml-agents/tests/trainers/test_bc.py
#	ml-agents/tests/trainers/test_ppo.py
#	ml-agents/tests/trainers/test_trainer_controller.py
@awjuliani awjuliani requested a review from vincentpierre March 11, 2019 22:23
@@ -3,15 +3,15 @@
# GRPC-TOOLS required. Install with `nuget install Grpc.Tools`.
# Then un-comment and replace [DIRECTORY] with location of files.
# For example, on macOS, you might have something like:
# COMPILER=Grpc.Tools.1.14.1/tools/macosx_x64
# COMPILER=Grpc.Tools.1.18.0/tools/macosx_x64
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason for this change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the new latest, so it is more likely that people on mac won't have to change the name here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it compatible with grpcio>=1.11.0,<1.12.0 on all 3 platforms ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It worked for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Linux - works as well.

@ervteng
Copy link
Contributor

ervteng commented Mar 15, 2019

I think we also should change the documentation (Installation.md) to have users install both ml-agents and ml-agents-envs. Once we push mlagents_envs to pypi, pip install -e . on ml-agents will work, but it might be confusing to people, as it'll pull a version that is independent of the repo they just cloned.

Otherwise functionally it seems to work OK!

@awjuliani
Copy link
Contributor Author

@ervteng Maybe we should tell people to just use the pypi packages, instead of installing from the repo source? That may be more straightforward for most users.

I also realized that we should test mlagents_envs on a python 3.7 environment, to ensure that it works there. If it does, we should change the requirements to allow it.

@harperj
Copy link
Contributor

harperj commented Mar 25, 2019

@awjuliani looks like this needs to be rebased for us to include it in v0.8 and tests fixed.

@ervteng
Copy link
Contributor

ervteng commented Mar 26, 2019

Also I realized we need to change the docs (the Readme in the ml-agents directory) to reflect the change.

@ervteng
Copy link
Contributor

ervteng commented Mar 26, 2019

@ervteng Maybe we should tell people to just use the pypi packages, instead of installing from the repo source? That may be more straightforward for most users.

We might be able to have our cake and eat it too... So for RLLib they use something called "extras" in the setup file. Example:

setup(
    name="Project-A",
    ...
    extras_require={
        'PDF':  ["ReportLab>=1.2", "RXP"],
        'reST': ["docutils>=0.3"],
    }
)

PDF and ReST are "extras" in this package (ours would be trainers, with envs being non-extra). We can specify different requirements for each. In this way, installing ml-agents would install just the envs package, and doing pip install ml-agents[trainers] would install both.

You could install using pip install -e ./ [trainers] as well. This way we'll have just one setup file and we won't need to change the directory structure or even the code's import statements.

@harperj harperj force-pushed the release-separate-package branch from 3dd47cd to 0cc1a6c Compare March 27, 2019 00:29
@Unity-Technologies Unity-Technologies deleted a comment Mar 27, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 27, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 27, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 27, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 27, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 27, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 28, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 28, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 28, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 28, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 28, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 28, 2019
@ervteng ervteng self-requested a review March 28, 2019 21:35
@Unity-Technologies Unity-Technologies deleted a comment Mar 28, 2019
@Unity-Technologies Unity-Technologies deleted a comment Mar 28, 2019
Ervin Teng added 5 commits March 28, 2019 15:55
# Conflicts:
#	ml-agents-envs/mlagents/envs/communicator_objects/agent_action_proto_pb2.py
#	ml-agents-envs/mlagents/envs/communicator_objects/agent_info_proto_pb2.py
#	ml-agents-envs/mlagents/envs/communicator_objects/environment_parameters_proto_pb2.py
@ervteng ervteng requested a review from vincentpierre March 29, 2019 00:22
@@ -0,0 +1,64 @@
# -*- coding: utf-8 -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should not be here, it should be with the others in ml-agents/ml-agents-envs/mlagents/envs/communicator_objects

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean the ones in ml-agents/mlagents right? I removed those three - but I think this one should be here.

@ervteng ervteng merged commit b1d7ac5 into develop Apr 1, 2019
@ervteng ervteng deleted the release-separate-package branch July 9, 2019 22:01
LeSphax pushed a commit to LeSphax/ml-agents-1 that referenced this pull request May 3, 2020
* Reogranize project

* Fix all tests

* Address comments

* Delete init file

* Update requirements

* Tick version

* Add timeout wait parameter (mlagents_envs) (Unity-Technologies#1699)

* Add timeout wait param

* Remove unnecessary function

* Add new meta files for communicator objects

* Fix all tests

* update circleci

* Reorganize mlagents_envs tests

* WIP: test removing circleci cache

* Move gym tests

* Namespaced packages

* Update installation instructions for separate packages

* Remove unused package from setup script

* Add Readme for ml-agents-envs

* Clarify docs and re-comment compiler in make.bat

* Add more doc to installation

* Add back fix for Hololens

* Recompile Protobufs

* Change mlagents_envs to mlagents.envs in trainer_controller

* Remove extraneous files, fix win bat script

* Support Python 3.7 for envs package
@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