From fe7bca1f62eb52f4b4138910fdc3c20206b47f58 Mon Sep 17 00:00:00 2001 From: Ervin Teng Date: Wed, 31 Jul 2019 16:51:32 -0700 Subject: [PATCH 1/5] Fix ml-agents doesn't quit from editor training --- ml-agents-envs/mlagents/envs/environment.py | 5 +++-- ml-agents-envs/mlagents/envs/exception.py | 8 ++++++++ .../mlagents/envs/subprocess_env_manager.py | 15 +++++++++++---- ml-agents/mlagents/trainers/trainer_controller.py | 7 +++++-- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/ml-agents-envs/mlagents/envs/environment.py b/ml-agents-envs/mlagents/envs/environment.py index b6acf890cd..d642d792b6 100644 --- a/ml-agents-envs/mlagents/envs/environment.py +++ b/ml-agents-envs/mlagents/envs/environment.py @@ -11,6 +11,7 @@ from .brain import AllBrainInfo, BrainInfo, BrainParameters from .exception import ( UnityEnvironmentException, + UnityCommunicationException, UnityActionException, UnityTimeOutException, ) @@ -343,7 +344,7 @@ def reset( self._generate_reset_input(train_mode, config, custom_reset_parameters) ) if outputs is None: - raise KeyboardInterrupt + raise UnityCommunicationException("Communicator has stopped.") rl_output = outputs.rl_output s = self._get_state(rl_output) self._global_done = s[1] @@ -570,7 +571,7 @@ def step( with hierarchical_timer("communicator.exchange"): outputs = self.communicator.exchange(step_input) if outputs is None: - raise KeyboardInterrupt + raise UnityCommunicationException("Communicator has stopped.") rl_output = outputs.rl_output state = self._get_state(rl_output) self._global_done = state[1] diff --git a/ml-agents-envs/mlagents/envs/exception.py b/ml-agents-envs/mlagents/envs/exception.py index 7824740c47..f1c0bed80c 100644 --- a/ml-agents-envs/mlagents/envs/exception.py +++ b/ml-agents-envs/mlagents/envs/exception.py @@ -19,6 +19,14 @@ class UnityEnvironmentException(UnityException): pass +class UnityCommunicationException(UnityException): + """ + Related to errors with the communicator. + """ + + pass + + class UnityActionException(UnityException): """ Related to errors with sending actions. diff --git a/ml-agents-envs/mlagents/envs/subprocess_env_manager.py b/ml-agents-envs/mlagents/envs/subprocess_env_manager.py index 679e548956..6a9dff1757 100644 --- a/ml-agents-envs/mlagents/envs/subprocess_env_manager.py +++ b/ml-agents-envs/mlagents/envs/subprocess_env_manager.py @@ -2,6 +2,7 @@ import cloudpickle from mlagents.envs import UnityEnvironment +from mlagents.envs.exception import UnityCommunicationException from multiprocessing import Process, Pipe, Queue from multiprocessing.connection import Connection from queue import Empty as EmptyQueueException @@ -47,14 +48,14 @@ def send(self, name: str, payload=None): cmd = EnvironmentCommand(name, payload) self.conn.send(cmd) except (BrokenPipeError, EOFError): - raise KeyboardInterrupt + raise UnityCommunicationException("UnityEnvironment worker: send failed.") def recv(self) -> EnvironmentResponse: try: response: EnvironmentResponse = self.conn.recv() return response except (BrokenPipeError, EOFError): - raise KeyboardInterrupt + raise UnityCommunicationException("UnityEnvironment worker: recv failed.") def close(self): try: @@ -115,10 +116,12 @@ def _send_response(cmd_name, payload): _send_response("global_done", env.global_done) elif cmd.name == "close": break - except KeyboardInterrupt: - print("UnityEnvironment worker: keyboard interrupt") + except (KeyboardInterrupt, UnityCommunicationException): + print("UnityEnvironment worker: environment stopping.") + step_queue.put(EnvironmentResponse("env_close", worker_id, None)) finally: step_queue.close() + step_queue.join_thread() env.close() @@ -171,6 +174,10 @@ def step(self) -> List[StepInfo]: try: while True: step = self.step_queue.get_nowait() + if step.name == "env_close": + raise UnityCommunicationException( + "At least one of the environments has closed." + ) self.env_workers[step.worker_id].waiting = False if step.worker_id not in step_workers: worker_steps.append(step) diff --git a/ml-agents/mlagents/trainers/trainer_controller.py b/ml-agents/mlagents/trainers/trainer_controller.py index 607fc4ede1..67adbc1488 100644 --- a/ml-agents/mlagents/trainers/trainer_controller.py +++ b/ml-agents/mlagents/trainers/trainer_controller.py @@ -14,7 +14,10 @@ from mlagents.envs import BrainParameters from mlagents.envs.env_manager import StepInfo from mlagents.envs.env_manager import EnvManager -from mlagents.envs.exception import UnityEnvironmentException +from mlagents.envs.exception import ( + UnityEnvironmentException, + UnityCommunicationException, +) from mlagents.envs.sampler_class import SamplerManager from mlagents.envs.timers import hierarchical_timer, get_timer_tree, timed from mlagents.trainers import Trainer, TrainerMetrics @@ -302,7 +305,7 @@ def start_learning( # Final save Tensorflow model if global_step != 0 and self.train_model: self._save_model() - except KeyboardInterrupt: + except (KeyboardInterrupt, UnityCommunicationException): if self.train_model: self._save_model_when_interrupted() pass From d22edac001b114f6cdb91c5463ef6f2ec6b56166 Mon Sep 17 00:00:00 2001 From: Ervin Teng Date: Wed, 31 Jul 2019 17:13:16 -0700 Subject: [PATCH 2/5] Remove join_thread --- ml-agents-envs/mlagents/envs/subprocess_env_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ml-agents-envs/mlagents/envs/subprocess_env_manager.py b/ml-agents-envs/mlagents/envs/subprocess_env_manager.py index 6a9dff1757..babb20382c 100644 --- a/ml-agents-envs/mlagents/envs/subprocess_env_manager.py +++ b/ml-agents-envs/mlagents/envs/subprocess_env_manager.py @@ -121,7 +121,6 @@ def _send_response(cmd_name, payload): step_queue.put(EnvironmentResponse("env_close", worker_id, None)) finally: step_queue.close() - step_queue.join_thread() env.close() From 4b03014f3b17511748b90df7ace48aa6e0dcc79a Mon Sep 17 00:00:00 2001 From: Jonathan Harper Date: Thu, 1 Aug 2019 11:20:18 -0700 Subject: [PATCH 3/5] Add SIGINT handler to prevent Windows failure to save --- .../mlagents/envs/subprocess_env_manager.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ml-agents-envs/mlagents/envs/subprocess_env_manager.py b/ml-agents-envs/mlagents/envs/subprocess_env_manager.py index babb20382c..2e8de0fe53 100644 --- a/ml-agents-envs/mlagents/envs/subprocess_env_manager.py +++ b/ml-agents-envs/mlagents/envs/subprocess_env_manager.py @@ -1,5 +1,6 @@ from typing import * import cloudpickle +import signal from mlagents.envs import UnityEnvironment from mlagents.envs.exception import UnityCommunicationException @@ -65,6 +66,15 @@ def close(self): self.process.join() +def _handle_sigint_with_keyboardinterrupt(): + """ Ensures SIGINT raised on Windows doesn't prevent us from cleaning up adequately. """ + + def signal_handler(signal, frame): + raise KeyboardInterrupt + + signal.signal(signal.SIGINT, signal_handler) + + def worker( parent_conn: Connection, step_queue: Queue, pickled_env_factory: str, worker_id: int ): @@ -72,6 +82,7 @@ def worker( pickled_env_factory ) env = env_factory(worker_id) + _handle_sigint_with_keyboardinterrupt() def _send_response(cmd_name, payload): parent_conn.send(EnvironmentResponse(cmd_name, worker_id, payload)) @@ -129,6 +140,7 @@ def __init__( self, env_factory: Callable[[int], BaseUnityEnvironment], n_env: int = 1 ): super().__init__() + _handle_sigint_with_keyboardinterrupt() self.env_workers: List[UnityEnvWorker] = [] self.step_queue: Queue = Queue() for worker_idx in range(n_env): From 1fccf1f73277ffafbfc054bb272280de9db13140 Mon Sep 17 00:00:00 2001 From: Jonathan Harper Date: Thu, 1 Aug 2019 14:13:38 -0700 Subject: [PATCH 4/5] Revert "Add SIGINT handler to prevent Windows failure to save" This reverts commit 4b03014f3b17511748b90df7ace48aa6e0dcc79a. --- .../mlagents/envs/subprocess_env_manager.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/ml-agents-envs/mlagents/envs/subprocess_env_manager.py b/ml-agents-envs/mlagents/envs/subprocess_env_manager.py index 2e8de0fe53..babb20382c 100644 --- a/ml-agents-envs/mlagents/envs/subprocess_env_manager.py +++ b/ml-agents-envs/mlagents/envs/subprocess_env_manager.py @@ -1,6 +1,5 @@ from typing import * import cloudpickle -import signal from mlagents.envs import UnityEnvironment from mlagents.envs.exception import UnityCommunicationException @@ -66,15 +65,6 @@ def close(self): self.process.join() -def _handle_sigint_with_keyboardinterrupt(): - """ Ensures SIGINT raised on Windows doesn't prevent us from cleaning up adequately. """ - - def signal_handler(signal, frame): - raise KeyboardInterrupt - - signal.signal(signal.SIGINT, signal_handler) - - def worker( parent_conn: Connection, step_queue: Queue, pickled_env_factory: str, worker_id: int ): @@ -82,7 +72,6 @@ def worker( pickled_env_factory ) env = env_factory(worker_id) - _handle_sigint_with_keyboardinterrupt() def _send_response(cmd_name, payload): parent_conn.send(EnvironmentResponse(cmd_name, worker_id, payload)) @@ -140,7 +129,6 @@ def __init__( self, env_factory: Callable[[int], BaseUnityEnvironment], n_env: int = 1 ): super().__init__() - _handle_sigint_with_keyboardinterrupt() self.env_workers: List[UnityEnvWorker] = [] self.step_queue: Queue = Queue() for worker_idx in range(n_env): From 4069615472cf08e6a951023b58534ce392bdb5e3 Mon Sep 17 00:00:00 2001 From: Jonathan Harper Date: Thu, 1 Aug 2019 14:14:08 -0700 Subject: [PATCH 5/5] Move EnvManager close to end of start_learning --- ml-agents/mlagents/trainers/trainer_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ml-agents/mlagents/trainers/trainer_controller.py b/ml-agents/mlagents/trainers/trainer_controller.py index 67adbc1488..cfa7911ae4 100644 --- a/ml-agents/mlagents/trainers/trainer_controller.py +++ b/ml-agents/mlagents/trainers/trainer_controller.py @@ -309,11 +309,11 @@ def start_learning( if self.train_model: self._save_model_when_interrupted() pass - env_manager.close() if self.train_model: self._write_training_metrics() self._export_graph() self._write_timing_tree() + env_manager.close() def end_trainer_episodes( self, env: BaseUnityEnvironment, lessons_incremented: Dict[str, bool]