From 2096a14898901047225da5f78b39b2c95bb21458 Mon Sep 17 00:00:00 2001 From: binliu Date: Mon, 1 Apr 2024 14:54:05 +0000 Subject: [PATCH 1/5] fix mlflow default behaviour Signed-off-by: binliu --- monai/bundle/utils.py | 6 +++--- monai/bundle/workflows.py | 24 ++++++++++++++---------- tests/test_fl_monai_algo.py | 6 +++--- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/monai/bundle/utils.py b/monai/bundle/utils.py index b187159c89..a0f39d236f 100644 --- a/monai/bundle/utils.py +++ b/monai/bundle/utils.py @@ -113,7 +113,7 @@ "experiment_name": "monai_experiment", "run_name": None, # may fill it at runtime - "execute_config": None, + "save_execute_config": True, "is_not_rank0": ( "$torch.distributed.is_available() \ and torch.distributed.is_initialized() and torch.distributed.get_rank() > 0" @@ -125,7 +125,7 @@ "tracking_uri": "@tracking_uri", "experiment_name": "@experiment_name", "run_name": "@run_name", - "artifacts": "@execute_config", + "artifacts": "@save_execute_config", "iteration_log": True, "epoch_log": True, "tag_name": "train_loss", @@ -148,7 +148,7 @@ "tracking_uri": "@tracking_uri", "experiment_name": "@experiment_name", "run_name": "@run_name", - "artifacts": "@execute_config", + "artifacts": "@save_execute_config", "iteration_log": False, "close_on_complete": True, }, diff --git a/monai/bundle/workflows.py b/monai/bundle/workflows.py index d876f6d7ae..22220c280c 100644 --- a/monai/bundle/workflows.py +++ b/monai/bundle/workflows.py @@ -506,13 +506,17 @@ def patch_bundle_tracking(parser: ConfigParser, settings: dict) -> None: parser[k] = v # save the executed config into file default_name = f"config_{time.strftime('%Y%m%d_%H%M%S')}.json" - filepath = parser.get("execute_config", None) - if filepath is None: - if "output_dir" not in parser: - # if no "output_dir" in the bundle config, default to "/eval" - parser["output_dir"] = f"{EXPR_KEY}{ID_REF_KEY}bundle_root + '/eval'" - # experiment management tools can refer to this config item to track the config info - parser["execute_config"] = parser["output_dir"] + f" + '/{default_name}'" - filepath = os.path.join(parser.get_parsed_content("output_dir"), default_name) - Path(filepath).parent.mkdir(parents=True, exist_ok=True) - parser.export_config_file(parser.get(), filepath) + filepath = parser.get("save_execute_config", True) + if filepath: + if isinstance(filepath, str): + Path(filepath).parent.mkdir(parents=True, exist_ok=True) + else: + if "output_dir" not in parser: + # if no "output_dir" in the bundle config, default to "/eval" + parser["output_dir"] = f"{EXPR_KEY}{ID_REF_KEY}bundle_root + '/eval'" + # experiment management tools can refer to this config item to track the config info + parser["save_execute_config"] = parser["output_dir"] + f" + '/{default_name}'" + filepath = os.path.join(parser.get_parsed_content("output_dir"), default_name) + parser.export_config_file(parser.get(), filepath) + else: + parser["save_execute_config"] = None diff --git a/tests/test_fl_monai_algo.py b/tests/test_fl_monai_algo.py index 54bec24b98..c8cb3451fc 100644 --- a/tests/test_fl_monai_algo.py +++ b/tests/test_fl_monai_algo.py @@ -75,7 +75,7 @@ tracking={ "handlers_id": DEFAULT_HANDLERS_ID, "configs": { - "execute_config": f"{_data_dir}/config_executed.json", + "save_execute_config": f"{_data_dir}/config_executed.json", "trainer": { "_target_": "MLFlowHandler", "tracking_uri": path_to_uri(_data_dir) + "/mlflow_override", @@ -201,7 +201,7 @@ def test_train(self, input_params): algo.finalize() # test experiment management - if "execute_config" in algo.train_workflow.parser: + if "save_execute_config" in algo.train_workflow.parser: self.assertTrue(os.path.exists(f"{_data_dir}/mlflow_override")) shutil.rmtree(f"{_data_dir}/mlflow_override") self.assertTrue(os.path.exists(f"{_data_dir}/config_executed.json")) @@ -224,7 +224,7 @@ def test_evaluate(self, input_params): algo.evaluate(data=data, extra={}) # test experiment management - if "execute_config" in algo.eval_workflow.parser: + if "save_execute_config" in algo.eval_workflow.parser: self.assertGreater(len(list(glob.glob(f"{_data_dir}/mlflow_*"))), 0) for f in list(glob.glob(f"{_data_dir}/mlflow_*")): shutil.rmtree(f) From 69f58af76fbf3c30ad1b6c537a71c24672b95974 Mon Sep 17 00:00:00 2001 From: binliu Date: Mon, 1 Apr 2024 15:01:48 +0000 Subject: [PATCH 2/5] add comment Signed-off-by: binliu --- monai/bundle/workflows.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/monai/bundle/workflows.py b/monai/bundle/workflows.py index 22220c280c..9c4acf9ee2 100644 --- a/monai/bundle/workflows.py +++ b/monai/bundle/workflows.py @@ -506,6 +506,9 @@ def patch_bundle_tracking(parser: ConfigParser, settings: dict) -> None: parser[k] = v # save the executed config into file default_name = f"config_{time.strftime('%Y%m%d_%H%M%S')}.json" + # Users can set the `save_execute_config` to `False`, `/path/to/artifacts` or `True`. + # If set to False, nothing will be recorded. If set to True, the default path will be logged. + # If set to a file path, the given path will be logged. filepath = parser.get("save_execute_config", True) if filepath: if isinstance(filepath, str): From ba88dc3e12fea3e3c705545118e1cb21f95f6b77 Mon Sep 17 00:00:00 2001 From: binliu Date: Mon, 1 Apr 2024 16:02:56 +0000 Subject: [PATCH 3/5] fix unit test Signed-off-by: binliu --- monai/bundle/workflows.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/monai/bundle/workflows.py b/monai/bundle/workflows.py index 9c4acf9ee2..471088994b 100644 --- a/monai/bundle/workflows.py +++ b/monai/bundle/workflows.py @@ -511,15 +511,14 @@ def patch_bundle_tracking(parser: ConfigParser, settings: dict) -> None: # If set to a file path, the given path will be logged. filepath = parser.get("save_execute_config", True) if filepath: - if isinstance(filepath, str): - Path(filepath).parent.mkdir(parents=True, exist_ok=True) - else: + if isinstance(filepath, bool): if "output_dir" not in parser: # if no "output_dir" in the bundle config, default to "/eval" parser["output_dir"] = f"{EXPR_KEY}{ID_REF_KEY}bundle_root + '/eval'" # experiment management tools can refer to this config item to track the config info parser["save_execute_config"] = parser["output_dir"] + f" + '/{default_name}'" filepath = os.path.join(parser.get_parsed_content("output_dir"), default_name) + Path(filepath).parent.mkdir(parents=True, exist_ok=True) parser.export_config_file(parser.get(), filepath) else: parser["save_execute_config"] = None From c330a77e76ff10738dcabd0b713ebf46ceb8b5fe Mon Sep 17 00:00:00 2001 From: binliu Date: Mon, 8 Apr 2024 16:19:09 +0000 Subject: [PATCH 4/5] update doc string Signed-off-by: binliu --- monai/bundle/scripts.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/monai/bundle/scripts.py b/monai/bundle/scripts.py index 2565a3cf64..61d9dc72b4 100644 --- a/monai/bundle/scripts.py +++ b/monai/bundle/scripts.py @@ -778,10 +778,19 @@ def run( https://docs.python.org/3/library/logging.config.html#logging.config.fileConfig. Default to None. tracking: if not None, enable the experiment tracking at runtime with optionally configurable and extensible. - if "mlflow", will add `MLFlowHandler` to the parsed bundle with default tracking settings, - if other string, treat it as file path to load the tracking settings. - if `dict`, treat it as tracking settings. - will patch the target config content with `tracking handlers` and the top-level items of `configs`. + If "mlflow", will add `MLFlowHandler` to the parsed bundle with default tracking settings where a set of + common parameters shown below will be added and can be passed through the `override` parameter of this method. + + - ``"output_dir"``: the path to save mlflow tracking outputs locally, default to "/eval". + - ``"tracking_uri"``: uri to save mlflow tracking outputs, default to "/output_dir/mlruns". + - ``"experiment_name"``: experiment name for this run, default to "monai_experiment". + - ``"run_name"``: the name of current run. + - ``"save_execute_config"``: whether to save the executed config files. It can be `False`, `/path/to/artifacts` + or `True`, default to `True`. + + If other string, treat it as file path to load the tracking settings. + If `dict`, treat it as tracking settings. + Will patch the target config content with `tracking handlers` and the top-level items of `configs`. for detailed usage examples, please check the tutorial: https://github.com/Project-MONAI/tutorials/blob/main/experiment_management/bundle_integrate_mlflow.ipynb. args_file: a JSON or YAML file to provide default values for `run_id`, `meta_file`, From 911fe4081651253ae59c06c6f0e1aad05b67b562 Mon Sep 17 00:00:00 2001 From: binliu Date: Wed, 10 Apr 2024 07:13:24 +0000 Subject: [PATCH 5/5] update the doc string Signed-off-by: binliu --- monai/bundle/scripts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monai/bundle/scripts.py b/monai/bundle/scripts.py index 61d9dc72b4..598d938cbd 100644 --- a/monai/bundle/scripts.py +++ b/monai/bundle/scripts.py @@ -786,7 +786,7 @@ def run( - ``"experiment_name"``: experiment name for this run, default to "monai_experiment". - ``"run_name"``: the name of current run. - ``"save_execute_config"``: whether to save the executed config files. It can be `False`, `/path/to/artifacts` - or `True`, default to `True`. + or `True`. If set to `True`, will save to the default path "/eval". Default to `True`. If other string, treat it as file path to load the tracking settings. If `dict`, treat it as tracking settings.