cleanup port logic in UnityEnvironment (#3673)

Author: Chris Elion

com.unity.ml-agents/CHANGELOG.md

@@ -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

docs/Custom-SideChannels.md

@@ -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])
 env.reset()
 string_log.send_string("The environment was reset")
 

docs/Python-API.md

@@ -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
@@ -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)
 
@@ -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)
 

gym-unity/gym_unity/envs/__init__.py

@@ -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
 

ml-agents-envs/mlagents_envs/communicator.py

@@ -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.
         :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:

ml-agents-envs/mlagents_envs/environment.py

@@ -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,
@@ -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.
@@ -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

ml-agents-envs/mlagents_envs/mock_communicator.py

@@ -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

ml-agents-envs/mlagents_envs/rpc_communicator.py

@@ -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

ml-agents-envs/mlagents_envs/tests/test_envs.py

@@ -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):

ml-agents/mlagents/trainers/learn.py

@@ -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",
     )