From 35b4522bff37f486a8e4b88daff92c4f9cf8a895 Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Fri, 9 Jun 2023 10:15:22 +0000 Subject: [PATCH 01/10] Capture failures from auto3dseg related subprocess calls Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- monai/apps/auto3dseg/auto_runner.py | 7 +++--- monai/apps/auto3dseg/bundle_gen.py | 4 ++-- monai/apps/auto3dseg/ensemble_builder.py | 5 ++--- monai/utils/__init__.py | 2 ++ monai/utils/misc.py | 28 ++++++++++++++++++++++++ 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/monai/apps/auto3dseg/auto_runner.py b/monai/apps/auto3dseg/auto_runner.py index eeaf9f5570..031f8358d1 100644 --- a/monai/apps/auto3dseg/auto_runner.py +++ b/monai/apps/auto3dseg/auto_runner.py @@ -13,7 +13,6 @@ import os import shutil -import subprocess import warnings from copy import deepcopy from time import sleep @@ -31,7 +30,7 @@ from monai.bundle import ConfigParser from monai.transforms import SaveImage from monai.utils import AlgoKeys, has_option, look_up_option, optional_import -from monai.utils.misc import check_kwargs_exist_in_class_init +from monai.utils.misc import check_kwargs_exist_in_class_init, run_cmd logger = get_logger(module_name=__name__) @@ -719,7 +718,7 @@ def _train_algo_in_nni(self, history: list[dict[str, Any]]) -> None: logger.info(f"AutoRunner HPO is in dry-run mode. Please manually launch: {cmd}") continue - subprocess.run(cmd.split(), check=True) + run_cmd(cmd.split(), check=True) n_trainings = len(import_bundle_algo_history(self.work_dir, only_trained=True)) while n_trainings - last_total_tasks < max_trial: @@ -727,7 +726,7 @@ def _train_algo_in_nni(self, history: list[dict[str, Any]]) -> None: n_trainings = len(import_bundle_algo_history(self.work_dir, only_trained=True)) cmd = "nnictl stop --all" - subprocess.run(cmd.split(), check=True) + run_cmd(cmd.split(), check=True) logger.info(f"NNI completes HPO on {name}") last_total_tasks = n_trainings diff --git a/monai/apps/auto3dseg/bundle_gen.py b/monai/apps/auto3dseg/bundle_gen.py index e949bcdac5..030ba787dc 100644 --- a/monai/apps/auto3dseg/bundle_gen.py +++ b/monai/apps/auto3dseg/bundle_gen.py @@ -33,7 +33,7 @@ from monai.auto3dseg.utils import algo_to_pickle from monai.bundle.config_parser import ConfigParser from monai.config import PathLike -from monai.utils import ensure_tuple +from monai.utils import ensure_tuple, run_cmd from monai.utils.enums import AlgoKeys logger = get_logger(module_name=__name__) @@ -243,7 +243,7 @@ def _run_cmd(self, cmd: str, devices_info: str = "") -> subprocess.CompletedProc logger.info(f"Launching: {' '.join(cmd_list)}") - return subprocess.run(cmd_list, env=ps_environ, check=True) + return run_cmd(cmd_list, env=ps_environ, check=True) def train( self, train_params: None | dict = None, device_setting: None | dict = None diff --git a/monai/apps/auto3dseg/ensemble_builder.py b/monai/apps/auto3dseg/ensemble_builder.py index 4163521af1..afb15d5d3e 100644 --- a/monai/apps/auto3dseg/ensemble_builder.py +++ b/monai/apps/auto3dseg/ensemble_builder.py @@ -12,7 +12,6 @@ from __future__ import annotations import os -import subprocess from abc import ABC, abstractmethod from collections.abc import Mapping, Sequence from copy import deepcopy @@ -33,7 +32,7 @@ from monai.transforms import MeanEnsemble, SaveImage, VoteEnsemble from monai.utils import RankFilter, deprecated_arg from monai.utils.enums import AlgoKeys -from monai.utils.misc import check_kwargs_exist_in_class_init, prob2class +from monai.utils.misc import check_kwargs_exist_in_class_init, prob2class, run_cmd from monai.utils.module import look_up_option, optional_import tqdm, has_tqdm = optional_import("tqdm", name="tqdm") @@ -672,5 +671,5 @@ def _create_cmd(self) -> None: cmd = f"{cmd} -m {base_cmd}" cmd_list = cmd.split() - _ = subprocess.run(cmd_list, env=ps_environ, check=True) + run_cmd(cmd_list, env=ps_environ, check=True) return diff --git a/monai/utils/__init__.py b/monai/utils/__init__.py index 4a8e439f0a..5fa62ed36b 100644 --- a/monai/utils/__init__.py +++ b/monai/utils/__init__.py @@ -66,6 +66,7 @@ MAX_SEED, ImageMetaKey, MONAIEnvVars, + check_kwargs_exist_in_class_init, check_parent_dir, copy_to_device, ensure_tuple, @@ -84,6 +85,7 @@ path_to_uri, pprint_edges, progress_bar, + run_cmd, sample_slices, save_obj, set_determinism, diff --git a/monai/utils/misc.py b/monai/utils/misc.py index 313d34c3fa..619317da6e 100644 --- a/monai/utils/misc.py +++ b/monai/utils/misc.py @@ -17,6 +17,7 @@ import pprint import random import shutil +import subprocess import tempfile import types import warnings @@ -73,6 +74,7 @@ "CheckKeyDuplicatesYamlLoader", "ConvertUnits", "check_kwargs_exist_in_class_init", + "run_cmd", ] _seed = None @@ -821,3 +823,29 @@ def check_kwargs_exist_in_class_init(cls, kwargs): extra_kwargs = input_kwargs - init_params return extra_kwargs == set(), extra_kwargs + + +def run_cmd(cmd_list, **kwargs) -> subprocess.CompletedProcess: + """ + Run a command by using ``subprocess.run`` with capture_output=True and stderr=subprocess.STDOUT + so that the raise exception will have that information. The argument `capture_output` can be set explicitly + if desired, but will be overriden with the debug status from the variable. + + Args: + cmd_list: a list of stringd describing the command to run. + kwargs: keyword arguments supported by the ``subprocess.run`` method. + + Returns: + a CompletedProcess instance after the command completes. + """ + debug = MONAIEnvVars.debug() + kwargs["capture_output"] = kwargs.get("capture_output", debug) + + try: + return subprocess.run(cmd_list, **kwargs) + except subprocess.CalledProcessError as e: + if not debug: + raise + output = repr(e.stdout).replace("\\n", "\n").replace("\\t", "\t") + errors = repr(e.stderr).replace("\\n", "\n").replace("\\t", "\t") + raise RuntimeError(f"subprocess call error {e.returncode}: {errors}, {output}.") From e0feb4350f43d094ab0442fb6186ad73e33a0a3d Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Mon, 12 Jun 2023 05:47:25 +0000 Subject: [PATCH 02/10] fix flake8 Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- monai/utils/misc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monai/utils/misc.py b/monai/utils/misc.py index 619317da6e..dc8af0ab8e 100644 --- a/monai/utils/misc.py +++ b/monai/utils/misc.py @@ -848,4 +848,4 @@ def run_cmd(cmd_list, **kwargs) -> subprocess.CompletedProcess: raise output = repr(e.stdout).replace("\\n", "\n").replace("\\t", "\t") errors = repr(e.stderr).replace("\\n", "\n").replace("\\t", "\t") - raise RuntimeError(f"subprocess call error {e.returncode}: {errors}, {output}.") + raise RuntimeError(f"subprocess call error {e.returncode}: {errors}, {output}.") from e From 85fe38682c141e3ae59c0f47f9b7fe6f8fa5b31d Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Mon, 12 Jun 2023 07:22:10 +0000 Subject: [PATCH 03/10] Fix mypy Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- monai/utils/misc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monai/utils/misc.py b/monai/utils/misc.py index dc8af0ab8e..6d122970db 100644 --- a/monai/utils/misc.py +++ b/monai/utils/misc.py @@ -825,14 +825,14 @@ def check_kwargs_exist_in_class_init(cls, kwargs): return extra_kwargs == set(), extra_kwargs -def run_cmd(cmd_list, **kwargs) -> subprocess.CompletedProcess: +def run_cmd(cmd_list: list[str], **kwargs: Any) -> subprocess.CompletedProcess: """ Run a command by using ``subprocess.run`` with capture_output=True and stderr=subprocess.STDOUT so that the raise exception will have that information. The argument `capture_output` can be set explicitly if desired, but will be overriden with the debug status from the variable. Args: - cmd_list: a list of stringd describing the command to run. + cmd_list: a list of strings describing the command to run. kwargs: keyword arguments supported by the ``subprocess.run`` method. Returns: From 09e55da44239a95c91b656894c0a6db5be9fb6ee Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Sun, 18 Jun 2023 14:40:03 +0000 Subject: [PATCH 04/10] Fix comments Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- monai/utils/misc.py | 4 ++-- tests/utils.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/monai/utils/misc.py b/monai/utils/misc.py index 6d122970db..de1059c41b 100644 --- a/monai/utils/misc.py +++ b/monai/utils/misc.py @@ -846,6 +846,6 @@ def run_cmd(cmd_list: list[str], **kwargs: Any) -> subprocess.CompletedProcess: except subprocess.CalledProcessError as e: if not debug: raise - output = repr(e.stdout).replace("\\n", "\n").replace("\\t", "\t") - errors = repr(e.stderr).replace("\\n", "\n").replace("\\t", "\t") + output = str(e.stdout.decode(errors="replace")) + errors = str(e.stderr.decode(errors="replace")) raise RuntimeError(f"subprocess call error {e.returncode}: {errors}, {output}.") from e diff --git a/tests/utils.py b/tests/utils.py index 1694ef5384..ecb3b77bff 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -819,10 +819,10 @@ def command_line_tests(cmd, copy_env=True): print(f"CUDA_VISIBLE_DEVICES in {__file__}", test_env.get("CUDA_VISIBLE_DEVICES")) try: normal_out = subprocess.run(cmd, env=test_env, check=True, capture_output=True) - print(repr(normal_out).replace("\\n", "\n").replace("\\t", "\t")) + print(str(normal_out.decode(errors="replace"))) except subprocess.CalledProcessError as e: - output = repr(e.stdout).replace("\\n", "\n").replace("\\t", "\t") - errors = repr(e.stderr).replace("\\n", "\n").replace("\\t", "\t") + output = str(e.stdout.decode(errors="replace")) + errors = str(e.stderr.deocde(errors="replace")) raise RuntimeError(f"subprocess call error {e.returncode}: {errors}, {output}") from e From 105fa7c661336525f52a27b32fe48f22bc689dd6 Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Mon, 19 Jun 2023 11:55:39 +0000 Subject: [PATCH 05/10] fix test error Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index ecb3b77bff..dc3f3f25e9 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -819,7 +819,7 @@ def command_line_tests(cmd, copy_env=True): print(f"CUDA_VISIBLE_DEVICES in {__file__}", test_env.get("CUDA_VISIBLE_DEVICES")) try: normal_out = subprocess.run(cmd, env=test_env, check=True, capture_output=True) - print(str(normal_out.decode(errors="replace"))) + print(str(repr(normal_out).decode(errors="replace"))) except subprocess.CalledProcessError as e: output = str(e.stdout.decode(errors="replace")) errors = str(e.stderr.deocde(errors="replace")) From 4c076ca99d8d90023730ddb7b5fb25fbc9f394de Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Mon, 19 Jun 2023 13:53:24 +0000 Subject: [PATCH 06/10] revert Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index dc3f3f25e9..f4d5c2185f 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -819,7 +819,7 @@ def command_line_tests(cmd, copy_env=True): print(f"CUDA_VISIBLE_DEVICES in {__file__}", test_env.get("CUDA_VISIBLE_DEVICES")) try: normal_out = subprocess.run(cmd, env=test_env, check=True, capture_output=True) - print(str(repr(normal_out).decode(errors="replace"))) + print(repr(normal_out).replace("\\n", "\n").replace("\\t", "\t")) except subprocess.CalledProcessError as e: output = str(e.stdout.decode(errors="replace")) errors = str(e.stderr.deocde(errors="replace")) From ccd33bdf06d7b64162d4ae6d121b814a0edd6535 Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Mon, 19 Jun 2023 14:02:46 +0000 Subject: [PATCH 07/10] revert Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- tests/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index f4d5c2185f..1694ef5384 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -821,8 +821,8 @@ def command_line_tests(cmd, copy_env=True): normal_out = subprocess.run(cmd, env=test_env, check=True, capture_output=True) print(repr(normal_out).replace("\\n", "\n").replace("\\t", "\t")) except subprocess.CalledProcessError as e: - output = str(e.stdout.decode(errors="replace")) - errors = str(e.stderr.deocde(errors="replace")) + output = repr(e.stdout).replace("\\n", "\n").replace("\\t", "\t") + errors = repr(e.stderr).replace("\\n", "\n").replace("\\t", "\t") raise RuntimeError(f"subprocess call error {e.returncode}: {errors}, {output}") from e From bfc001a3c6704c2d4323e7a19bed2d3cd7779179 Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Mon, 19 Jun 2023 14:56:41 +0000 Subject: [PATCH 08/10] add test Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- tests/test_monai_utils_misc.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/test_monai_utils_misc.py b/tests/test_monai_utils_misc.py index ef135c45b1..a110ba16fd 100644 --- a/tests/test_monai_utils_misc.py +++ b/tests/test_monai_utils_misc.py @@ -15,7 +15,7 @@ from parameterized import parameterized -from monai.utils.misc import check_kwargs_exist_in_class_init, to_tuple_of_dictionaries +from monai.utils.misc import check_kwargs_exist_in_class_init, to_tuple_of_dictionaries, run_cmd TO_TUPLE_OF_DICTIONARIES_TEST_CASES = [ ({}, tuple(), tuple()), @@ -71,6 +71,19 @@ def test_kwargs(self): def _custom_user_function(self, cls, *args, **kwargs): return check_kwargs_exist_in_class_init(cls, kwargs) +class TestCommandRunner(unittest.TestCase): + def test_run_cmd(self): + cmd1 = "python" + cmd2 = "-c" + cmd3 = "import sys; print(\"\\tThis is on stderr\\n\", file=sys.stderr); sys.exit(1)" + import os + os.environ["MONAI_DEBUG"] = str(True) + try: + run_cmd([cmd1, cmd2, cmd3], check=True) + except RuntimeError as err: + self.assertIn('This is on stderr', str(err)) + self.assertNotIn('\\n', str(err)) + self.assertNotIn('\\t', str(err)) if __name__ == "__main__": unittest.main() From 2eefc05bf108a546a99ce1bdfa71e06801521534 Mon Sep 17 00:00:00 2001 From: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> Date: Mon, 19 Jun 2023 14:57:23 +0000 Subject: [PATCH 09/10] autofix Signed-off-by: Mingxin <18563433+mingxin-zheng@users.noreply.github.com> --- tests/test_monai_utils_misc.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_monai_utils_misc.py b/tests/test_monai_utils_misc.py index a110ba16fd..b6cd15db7a 100644 --- a/tests/test_monai_utils_misc.py +++ b/tests/test_monai_utils_misc.py @@ -11,11 +11,12 @@ from __future__ import annotations +import os import unittest from parameterized import parameterized -from monai.utils.misc import check_kwargs_exist_in_class_init, to_tuple_of_dictionaries, run_cmd +from monai.utils.misc import check_kwargs_exist_in_class_init, run_cmd, to_tuple_of_dictionaries TO_TUPLE_OF_DICTIONARIES_TEST_CASES = [ ({}, tuple(), tuple()), @@ -71,19 +72,20 @@ def test_kwargs(self): def _custom_user_function(self, cls, *args, **kwargs): return check_kwargs_exist_in_class_init(cls, kwargs) + class TestCommandRunner(unittest.TestCase): def test_run_cmd(self): cmd1 = "python" cmd2 = "-c" - cmd3 = "import sys; print(\"\\tThis is on stderr\\n\", file=sys.stderr); sys.exit(1)" - import os + cmd3 = 'import sys; print("\\tThis is on stderr\\n", file=sys.stderr); sys.exit(1)' os.environ["MONAI_DEBUG"] = str(True) try: run_cmd([cmd1, cmd2, cmd3], check=True) except RuntimeError as err: - self.assertIn('This is on stderr', str(err)) - self.assertNotIn('\\n', str(err)) - self.assertNotIn('\\t', str(err)) + self.assertIn("This is on stderr", str(err)) + self.assertNotIn("\\n", str(err)) + self.assertNotIn("\\t", str(err)) + if __name__ == "__main__": unittest.main() From dee12cc5cc618a3598ea1cadec0b1f4434c4a4a1 Mon Sep 17 00:00:00 2001 From: Wenqi Li Date: Wed, 21 Jun 2023 23:12:01 +0100 Subject: [PATCH 10/10] resume debug flag Signed-off-by: Wenqi Li --- tests/test_monai_utils_misc.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_monai_utils_misc.py b/tests/test_monai_utils_misc.py index b6cd15db7a..b1e4ab4b12 100644 --- a/tests/test_monai_utils_misc.py +++ b/tests/test_monai_utils_misc.py @@ -74,6 +74,15 @@ def _custom_user_function(self, cls, *args, **kwargs): class TestCommandRunner(unittest.TestCase): + def setUp(self): + self.orig_flag = os.environ.get("MONAI_DEBUG") + + def tearDown(self): + if self.orig_flag is not None: + os.environ["MONAI_DEBUG"] = self.orig_flag + else: + os.environ.pop("MONAI_DEBUG") + def test_run_cmd(self): cmd1 = "python" cmd2 = "-c"