Repository: Project-MONAI/MONAI

Issue:
Enhance MLFlowHandler
**Is your feature request related to a problem? Please describe.**

**Describe the solution you'd like**

MLFlowHandler is being used to log DL experiments with PyTorch Ignite for MONAI bundles. We can further enhance the module by adding these four features:
 P0:
- API for users to add experiment/run name in MLFlow
- API for users to log customized params for each run
- Methods to log json file content
- Methods to log result images
- Methods to log optimizer params 
- Make sure this handler works in multi-gpu & multi-node environment
- Make sure this handler works in all existed bundles

P1:
- Methods to log dataset
- Enhance the `tracking_uri` parameter for convenient modification
- 

**Describe alternatives you've considered**
Use `ignite.contrib.handlers.mlflow_logger`

https://pytorch.org/ignite/generated/ignite.contrib.handlers.mlflow_logger.html#module-ignite.contrib.handlers.mlflow_logger



Discussion:

Patch:
diff --git a/monai/bundle/__init__.py b/monai/bundle/__init__.py
--- a/monai/bundle/__init__.py
+++ b/monai/bundle/__init__.py
@@ -25,4 +25,12 @@
     verify_metadata,
     verify_net_in_out,
 )
-from .utils import DEFAULT_EXP_MGMT_SETTINGS, EXPR_KEY, ID_REF_KEY, ID_SEP_KEY, MACRO_KEY, load_bundle_config
+from .utils import (
+    DEFAULT_EXP_MGMT_SETTINGS,
+    DEFAULT_MLFLOW_SETTINGS,
+    EXPR_KEY,
+    ID_REF_KEY,
+    ID_SEP_KEY,
+    MACRO_KEY,
+    load_bundle_config,
+)
diff --git a/monai/bundle/scripts.py b/monai/bundle/scripts.py
--- a/monai/bundle/scripts.py
+++ b/monai/bundle/scripts.py
@@ -547,17 +547,36 @@ def run(
                     },
                     "configs": {
                         "tracking_uri": "<path>",
+                        "experiment_name": "monai_experiment",
+                        "run_name": None,
+                        "is_not_rank0": (
+                            "$torch.distributed.is_available() \
+                                and torch.distributed.is_initialized() and torch.distributed.get_rank() > 0"
+                        ),
                         "trainer": {
                             "_target_": "MLFlowHandler",
+                            "_disabled_": "@is_not_rank0",
                             "tracking_uri": "@tracking_uri",
+                            "experiment_name": "@experiment_name",
+                            "run_name": "@run_name",
                             "iteration_log": True,
                             "output_transform": "$monai.handlers.from_engine(['loss'], first=True)",
                         },
                         "validator": {
-                            "_target_": "MLFlowHandler", "tracking_uri": "@tracking_uri", "iteration_log": False,
+                            "_target_": "MLFlowHandler",
+                            "_disabled_": "@is_not_rank0",
+                            "tracking_uri": "@tracking_uri",
+                            "experiment_name": "@experiment_name",
+                            "run_name": "@run_name",
+                            "iteration_log": False,
                         },
                         "evaluator": {
-                            "_target_": "MLFlowHandler", "tracking_uri": "@tracking_uri", "iteration_log": False,
+                            "_target_": "MLFlowHandler",
+                            "_disabled_": "@is_not_rank0",
+                            "tracking_uri": "@tracking_uri",
+                            "experiment_name": "@experiment_name",
+                            "run_name": "@run_name",
+                            "iteration_log": False,
                         },
                     },
                 },
diff --git a/monai/bundle/utils.py b/monai/bundle/utils.py
--- a/monai/bundle/utils.py
+++ b/monai/bundle/utils.py
@@ -19,7 +19,7 @@
 
 yaml, _ = optional_import("yaml")
 
-__all__ = ["ID_REF_KEY", "ID_SEP_KEY", "EXPR_KEY", "MACRO_KEY"]
+__all__ = ["ID_REF_KEY", "ID_SEP_KEY", "EXPR_KEY", "MACRO_KEY", "DEFAULT_MLFLOW_SETTINGS", "DEFAULT_EXP_MGMT_SETTINGS"]
 
 ID_REF_KEY = "@"  # start of a reference to a ConfigItem
 ID_SEP_KEY = "#"  # separator for the ID of a ConfigItem
@@ -105,19 +105,42 @@
     "handlers_id": DEFAULT_HANDLERS_ID,
     "configs": {
         "tracking_uri": "$@output_dir + '/mlruns'",
+        "experiment_name": "monai_experiment",
+        "run_name": None,
+        "is_not_rank0": (
+            "$torch.distributed.is_available() \
+                and torch.distributed.is_initialized() and torch.distributed.get_rank() > 0"
+        ),
         # MLFlowHandler config for the trainer
         "trainer": {
             "_target_": "MLFlowHandler",
+            "_disabled_": "@is_not_rank0",
             "tracking_uri": "@tracking_uri",
+            "experiment_name": "@experiment_name",
+            "run_name": "@run_name",
             "iteration_log": True,
             "epoch_log": True,
             "tag_name": "train_loss",
             "output_transform": "$monai.handlers.from_engine(['loss'], first=True)",
         },
         # MLFlowHandler config for the validator
-        "validator": {"_target_": "MLFlowHandler", "tracking_uri": "@tracking_uri", "iteration_log": False},
+        "validator": {
+            "_target_": "MLFlowHandler",
+            "_disabled_": "@is_not_rank0",
+            "tracking_uri": "@tracking_uri",
+            "experiment_name": "@experiment_name",
+            "run_name": "@run_name",
+            "iteration_log": False,
+        },
         # MLFlowHandler config for the evaluator
-        "evaluator": {"_target_": "MLFlowHandler", "tracking_uri": "@tracking_uri", "iteration_log": False},
+        "evaluator": {
+            "_target_": "MLFlowHandler",
+            "_disabled_": "@is_not_rank0",
+            "tracking_uri": "@tracking_uri",
+            "experiment_name": "@experiment_name",
+            "run_name": "@run_name",
+            "iteration_log": False,
+        },
     },
 }
 
diff --git a/monai/handlers/mlflow_handler.py b/monai/handlers/mlflow_handler.py
--- a/monai/handlers/mlflow_handler.py
+++ b/monai/handlers/mlflow_handler.py
@@ -9,12 +9,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import TYPE_CHECKING, Any, Callable, Optional, Sequence
+import os
+import time
+from pathlib import Path
+from typing import TYPE_CHECKING, Any, Callable, Dict, Optional, Sequence, Union
 
 import torch
 
 from monai.config import IgniteInfo
-from monai.utils import min_version, optional_import
+from monai.utils import ensure_tuple, min_version, optional_import
 
 Events, _ = optional_import("ignite.engine", IgniteInfo.OPT_IMPORT_VERSION, min_version, "Events")
 mlflow, _ = optional_import("mlflow")
@@ -72,11 +75,21 @@ class MLFlowHandler:
         state_attributes: expected attributes from `engine.state`, if provided, will extract them
             when epoch completed.
         tag_name: when iteration output is a scalar, `tag_name` is used to track, defaults to `'Loss'`.
+        experiment_name: name for an experiment, defaults to `default_experiment`.
+        run_name: name for run in an experiment.
+        experiment_param: a dict recording parameters which will not change through whole experiment,
+            like torch version, cuda version and so on.
+        artifacts: paths to images that need to be recorded after a whole run.
+        optimizer_param_names: parameters' name in optimizer that need to be record during runing,
+            defaults to "lr".
 
     For more details of MLFlow usage, please refer to: https://mlflow.org/docs/latest/index.html.
 
     """
 
+    # parameters that are logged at the start of training
+    default_tracking_params = ["max_epochs", "epoch_length"]
+
     def __init__(
         self,
         tracking_uri: Optional[str] = None,
@@ -88,6 +101,11 @@ def __init__(
         global_epoch_transform: Callable = lambda x: x,
         state_attributes: Optional[Sequence[str]] = None,
         tag_name: str = DEFAULT_TAG,
+        experiment_name: str = "default_experiment",
+        run_name: Optional[str] = None,
+        experiment_param: Optional[Dict] = None,
+        artifacts: Optional[Union[str, Sequence[Path]]] = None,
+        optimizer_param_names: Union[str, Sequence[str]] = "lr",
     ) -> None:
         if tracking_uri is not None:
             mlflow.set_tracking_uri(tracking_uri)
@@ -100,6 +118,27 @@ def __init__(
         self.global_epoch_transform = global_epoch_transform
         self.state_attributes = state_attributes
         self.tag_name = tag_name
+        self.experiment_name = experiment_name
+        self.run_name = run_name
+        self.experiment_param = experiment_param
+        self.artifacts = ensure_tuple(artifacts)
+        self.optimizer_param_names = ensure_tuple(optimizer_param_names)
+        self.client = mlflow.MlflowClient()
+
+    def _delete_exist_param_in_dict(self, param_dict: Dict) -> None:
+        """
+        Delete parameters in given dict, if they are already logged by current mlflow run.
+
+        Args:
+            param_dict: parameter dict to be logged to mlflow.
+        """
+        key_list = list(param_dict.keys())
+        cur_run = mlflow.active_run()
+        log_data = self.client.get_run(cur_run.info.run_id).data
+        log_param_dict = log_data.params
+        for key in key_list:
+            if key in log_param_dict:
+                del param_dict[key]
 
     def attach(self, engine: Engine) -> None:
         """
@@ -115,14 +154,53 @@ def attach(self, engine: Engine) -> None:
             engine.add_event_handler(Events.ITERATION_COMPLETED, self.iteration_completed)
         if self.epoch_log and not engine.has_event_handler(self.epoch_completed, Events.EPOCH_COMPLETED):
             engine.add_event_handler(Events.EPOCH_COMPLETED, self.epoch_completed)
+        if not engine.has_event_handler(self.complete, Events.COMPLETED):
+            engine.add_event_handler(Events.COMPLETED, self.complete)
 
-    def start(self) -> None:
+    def start(self, engine: Engine) -> None:
         """
         Check MLFlow status and start if not active.
 
         """
+        mlflow.set_experiment(self.experiment_name)
         if mlflow.active_run() is None:
-            mlflow.start_run()
+            run_name = f"run_{time.strftime('%Y%m%d_%H%M%S')}" if self.run_name is None else self.run_name
+            mlflow.start_run(run_name=run_name)
+
+        if self.experiment_param:
+            mlflow.log_params(self.experiment_param)
+
+        attrs = {attr: getattr(engine.state, attr, None) for attr in self.default_tracking_params}
+        self._delete_exist_param_in_dict(attrs)
+        mlflow.log_params(attrs)
+
+    def _parse_artifacts(self):
+        """
+        Log artifacts to mlflow. Given a path, all files in the path will be logged recursively.
+        Given a file, it will be logged to mlflow.
+        """
+        artifact_list = []
+        for path_name in self.artifacts:
+            # in case the input is (None,) by default
+            if not path_name:
+                continue
+            if os.path.isfile(path_name):
+                artifact_list.append(path_name)
+            else:
+                for root, _, filenames in os.walk(path_name):
+                    for filename in filenames:
+                        file_path = os.path.join(root, filename)
+                        artifact_list.append(file_path)
+        return artifact_list
+
+    def complete(self) -> None:
+        """
+        Handler for train or validation/evaluation completed Event.
+        """
+        if self.artifacts:
+            artifact_list = self._parse_artifacts()
+            for artifact in artifact_list:
+                mlflow.log_artifact(artifact)
 
     def close(self) -> None:
         """
@@ -199,3 +277,13 @@ def _default_iteration_log(self, engine: Engine) -> None:
             loss = {self.tag_name: loss.item() if isinstance(loss, torch.Tensor) else loss}
 
         mlflow.log_metrics(loss, step=engine.state.iteration)
+
+        # If there is optimizer attr in engine, then record parameters specified in init function.
+        if hasattr(engine, "optimizer"):
+            cur_optimizer = engine.optimizer  # type: ignore
+            for param_name in self.optimizer_param_names:
+                params = {
+                    f"{param_name} group_{i}": float(param_group[param_name])
+                    for i, param_group in enumerate(cur_optimizer.param_groups)
+                }
+                mlflow.log_metrics(params, step=engine.state.iteration)


Test patch:
diff --git a/tests/test_handler_mlflow.py b/tests/test_handler_mlflow.py
--- a/tests/test_handler_mlflow.py
+++ b/tests/test_handler_mlflow.py
@@ -15,6 +15,7 @@
 import unittest
 from pathlib import Path
 
+import numpy as np
 from ignite.engine import Engine, Events
 
 from monai.handlers import MLFlowHandler
@@ -22,6 +23,7 @@
 
 class TestHandlerMLFlow(unittest.TestCase):
     def test_metrics_track(self):
+        experiment_param = {"backbone": "efficientnet_b0"}
         with tempfile.TemporaryDirectory() as tempdir:
 
             # set up engine
@@ -39,8 +41,18 @@ def _update_metric(engine):
 
             # set up testing handler
             test_path = os.path.join(tempdir, "mlflow_test")
+            artifact_path = os.path.join(tempdir, "artifacts")
+            os.makedirs(artifact_path, exist_ok=True)
+            dummy_numpy = np.zeros((64, 64, 3))
+            dummy_path = os.path.join(artifact_path, "tmp.npy")
+            np.save(dummy_path, dummy_numpy)
             handler = MLFlowHandler(
-                iteration_log=False, epoch_log=True, tracking_uri=Path(test_path).as_uri(), state_attributes=["test"]
+                iteration_log=False,
+                epoch_log=True,
+                tracking_uri=Path(test_path).as_uri(),
+                state_attributes=["test"],
+                experiment_param=experiment_param,
+                artifacts=[artifact_path],
             )
             handler.attach(engine)
             engine.run(range(3), max_epochs=2)
diff --git a/tests/test_scale_intensity_range_percentiles.py b/tests/test_scale_intensity_range_percentiles.py
--- a/tests/test_scale_intensity_range_percentiles.py
+++ b/tests/test_scale_intensity_range_percentiles.py
@@ -58,7 +58,7 @@ def test_relative_scaling(self):
         for p in TEST_NDARRAYS:
             result = scaler(p(img))
             assert_allclose(
-                result, p(np.clip(expected_img, expected_b_min, expected_b_max)), type_test="tensor", rtol=1e-4
+                result, p(np.clip(expected_img, expected_b_min, expected_b_max)), type_test="tensor", rtol=0.1
             )
 
     def test_invalid_instantiation(self):