-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Conversation
Hotfix 0.6.0a to master
* 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
protobuf-definitions/make.bat
Outdated
@@ -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 |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It worked for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linux - works as well.
I think we also should change the documentation (Installation.md) to have users install both Otherwise functionally it seems to work OK! |
@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 |
@awjuliani looks like this needs to be rebased for us to include it in v0.8 and tests fixed. |
Also I realized we need to change the docs (the Readme in the ml-agents directory) to reflect the change. |
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:
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. |
3dd47cd
to
0cc1a6c
Compare
UnitySDK/Assets/ML-Agents/Scripts/CommunicatorObjects/UnityToExternalGrpc.cs
Outdated
Show resolved
Hide resolved
# 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
@@ -0,0 +1,64 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
* 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
This PR splits the
mlagents
package into to packages. One ismlagents_envs
which contains only the python api, and is nice and lightweight. The second ismlagents
which contains the PPO/BC trainer and depends onmlagents_envs
.