Skip to content

cleanup port logic in UnityEnvironment #3673

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 3 commits into from
Mar 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions com.unity.ml-agents/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Format of console output has changed slightly and now matches the name of the model/summary directory. (#3630, #3616)
- Raise the wall in CrawlerStatic scene to prevent Agent from falling off. (#3650)
- Renamed 'Generalization' feature to 'Environment Parameter Randomization'.
- The way that UnityEnvironment decides the port was changed. If no port is specified, the behavior will depend on the `file_name` parameter. If it is `None`, 5004 (the editor port) will be used; otherwise 5005 (the base environment port) will be used.

## [0.15.0-preview] - 2020-03-18
### Major Changes
Expand Down
2 changes: 1 addition & 1 deletion docs/Custom-SideChannels.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ messages to the Unity environment from Python using it.
string_log = StringLogChannel()

# We start the communication with the Unity Editor and pass the string_log side channel as input
env = UnityEnvironment(base_port=UnityEnvironment.DEFAULT_EDITOR_PORT, side_channels=[string_log])
env = UnityEnvironment(side_channels=[string_log])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since these should now do the correct thing, I removed the explicit port (for simplicity and future-proofing) wherever I found it in the docs.

env.reset()
string_log.send_string("The environment was reset")

Expand Down
6 changes: 3 additions & 3 deletions docs/Python-API.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ as `envs`. For example, if the filename of your Unity environment is `3DBall`, i

```python
from mlagents_envs.environment import UnityEnvironment
env = UnityEnvironment(file_name="3DBall", base_port=5005, seed=1, side_channels=[])
env = UnityEnvironment(file_name="3DBall", seed=1, side_channels=[])
```

- `file_name` is the name of the environment binary (located in the root
Expand Down Expand Up @@ -239,7 +239,7 @@ from mlagents_envs.side_channel.engine_configuration_channel import EngineConfig

channel = EngineConfigurationChannel()

env = UnityEnvironment(base_port = UnityEnvironment.DEFAULT_EDITOR_PORT, side_channels = [channel])
env = UnityEnvironment(side_channels=[channel])

channel.set_configuration_parameters(time_scale = 2.0)

Expand Down Expand Up @@ -267,7 +267,7 @@ from mlagents_envs.side_channel.float_properties_channel import FloatPropertiesC

channel = FloatPropertiesChannel()

env = UnityEnvironment(base_port = UnityEnvironment.DEFAULT_EDITOR_PORT, side_channels = [channel])
env = UnityEnvironment(side_channels=[channel])

channel.set_property("parameter_1", 2.0)

Expand Down
2 changes: 1 addition & 1 deletion gym-unity/gym_unity/envs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def __init__(
:param no_graphics: Whether to run the Unity simulator in no-graphics mode
:param allow_multiple_visual_obs: If True, return a list of visual observations instead of only one.
"""
base_port = 5005
base_port = UnityEnvironment.BASE_ENVIRONMENT_PORT
if environment_filename is None:
base_port = UnityEnvironment.DEFAULT_EDITOR_PORT

Expand Down
2 changes: 1 addition & 1 deletion ml-agents-envs/mlagents_envs/communicator.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ def __init__(self, worker_id=0, base_port=5005):
"""
Python side of the communication. Must be used in pair with the right Unity Communicator equivalent.

:int worker_id: Offset from base_port. Used for training multiple environments simultaneously.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rearranged to match the argument order. I think I changed the wording of this wherever I saw it (git blame showed it was 3 years old).

:int base_port: Baseline port number to connect to Unity environment over. worker_id increments over this.
:int worker_id: Number to add to communication port (5005) [0]. Used for asynchronous agent scenarios.
"""

def initialize(self, inputs: UnityInputProto) -> UnityOutputProto:
Expand Down
15 changes: 13 additions & 2 deletions ml-agents-envs/mlagents_envs/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,18 @@ class UnityEnvironment(BaseEnv):
# isn't specified, this port will be used.
DEFAULT_EDITOR_PORT = 5004

# Default base port for environments. Each environment will be offset from this
# by it's worker_id.
BASE_ENVIRONMENT_PORT = 5005

# Command line argument used to pass the port to the executable environment.
PORT_COMMAND_LINE_ARG = "--mlagents-port"

def __init__(
self,
file_name: Optional[str] = None,
worker_id: int = 0,
base_port: int = 5005,
base_port: Optional[int] = None,
seed: int = 0,
docker_training: bool = False,
no_graphics: bool = False,
Expand All @@ -87,7 +91,8 @@ def __init__(

:string file_name: Name of Unity environment binary.
:int base_port: Baseline port number to connect to Unity environment over. worker_id increments over this.
:int worker_id: Number to add to communication port (5005) [0]. Used for asynchronous agent scenarios.
If no environment is specified (i.e. file_name is None), the DEFAULT_EDITOR_PORT will be used.
:int worker_id: Offset from base_port. Used for training multiple environments simultaneously.
:bool docker_training: Informs this class whether the process is being run within a container.
:bool no_graphics: Whether to run the Unity simulator in no-graphics mode
:int timeout_wait: Time (in seconds) to wait for connection from environment.
Expand All @@ -96,6 +101,12 @@ def __init__(
"""
args = args or []
atexit.register(self._close)
# If base port is not specified, use BASE_ENVIRONMENT_PORT if we have
# an environment, otherwise DEFAULT_EDITOR_PORT
if base_port is None:
base_port = (
self.BASE_ENVIRONMENT_PORT if file_name else self.DEFAULT_EDITOR_PORT
)
self.port = base_port + worker_id
self._buffer_size = 12000
# If true, this means the environment was successfully loaded
Expand Down
3 changes: 0 additions & 3 deletions ml-agents-envs/mlagents_envs/mock_communicator.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ def __init__(
):
"""
Python side of the grpc communication. Python is the client and Unity the server

:int base_port: Baseline port number to connect to Unity environment over. worker_id increments over this.
:int worker_id: Number to add to communication port (5005) [0]. Used for asynchronous agent scenarios.
"""
super().__init__()
self.is_discrete = discrete_action
Expand Down
3 changes: 2 additions & 1 deletion ml-agents-envs/mlagents_envs/rpc_communicator.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ def __init__(self, worker_id=0, base_port=5005, timeout_wait=30):


:int base_port: Baseline port number to connect to Unity environment over. worker_id increments over this.
:int worker_id: Number to add to communication port (5005) [0]. Used for asynchronous agent scenarios.
:int worker_id: Offset from base_port. Used for training multiple environments simultaneously.
:int timeout_wait: Timeout (in seconds) to wait for a response before exiting.
"""
super().__init__(worker_id, base_port)
self.port = base_port + worker_id
Expand Down
23 changes: 23 additions & 0 deletions ml-agents-envs/mlagents_envs/tests/test_envs.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,29 @@ def test_initialization(mock_communicator, mock_launcher):
env.close()


@pytest.mark.parametrize(
"base_port,file_name,expected",
[
# Non-None base port value will always be used
(6001, "foo.exe", 6001),
# No port specified and environment specified, so use BASE_ENVIRONMENT_PORT
(None, "foo.exe", UnityEnvironment.BASE_ENVIRONMENT_PORT),
# No port specified and no environment, so use DEFAULT_EDITOR_PORT
(None, None, UnityEnvironment.DEFAULT_EDITOR_PORT),
],
)
@mock.patch("mlagents_envs.environment.UnityEnvironment.executable_launcher")
@mock.patch("mlagents_envs.environment.UnityEnvironment.get_communicator")
def test_port_defaults(
mock_communicator, mock_launcher, base_port, file_name, expected
):
mock_communicator.return_value = MockCommunicator(
discrete_action=False, visual_inputs=0
)
env = UnityEnvironment(file_name=file_name, worker_id=0, base_port=base_port)
assert expected == env.port


@mock.patch("mlagents_envs.environment.UnityEnvironment.executable_launcher")
@mock.patch("mlagents_envs.environment.UnityEnvironment.get_communicator")
def test_reset(mock_communicator, mock_launcher):
Expand Down
2 changes: 1 addition & 1 deletion ml-agents/mlagents/trainers/learn.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def _create_parser():
)
argparser.add_argument(
"--base-port",
default=5005,
default=UnityEnvironment.BASE_ENVIRONMENT_PORT,
type=int,
help="Base port for environment communication",
)
Expand Down