From 1d4d038ef8da22fb233d970a0e8c48998bad0786 Mon Sep 17 00:00:00 2001 From: vfdev-5 Date: Fri, 10 Feb 2023 10:30:53 +0100 Subject: [PATCH 1/5] Updated tb stats handler iter/epoch args --- monai/handlers/tensorboard_handlers.py | 33 ++++++++-- tests/test_handler_tb_stats.py | 88 ++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/monai/handlers/tensorboard_handlers.py b/monai/handlers/tensorboard_handlers.py index b4cba4ee50..1cff275866 100644 --- a/monai/handlers/tensorboard_handlers.py +++ b/monai/handlers/tensorboard_handlers.py @@ -91,8 +91,8 @@ def __init__( self, summary_writer: SummaryWriter | SummaryWriterX | None = None, log_dir: str = "./runs", - iteration_log: bool = True, - epoch_log: bool = True, + iteration_log: bool | Callable[[Engine, int], bool] = True, + epoch_log: bool | Callable[[Engine, int], bool] = True, epoch_event_writer: Callable[[Engine, Any], Any] | None = None, epoch_interval: int = 1, iteration_event_writer: Callable[[Engine, Any], Any] | None = None, @@ -108,13 +108,20 @@ def __init__( default to create a new TensorBoard writer. log_dir: if using default SummaryWriter, write logs to this directory, default is `./runs`. iteration_log: whether to write data to TensorBoard when iteration completed, default to `True`. + ``iteration_log`` can be also a function and it will be interpreted as an event filter + (see https://pytorch.org/ignite/generated/ignite.engine.events.Events.html for details). + Event filter function accepts as input engine and event value (iteration) and should return True/False. epoch_log: whether to write data to TensorBoard when epoch completed, default to `True`. + ``epoch_log`` can be also a function and it will be interpreted as an event filter. + See ``iteration_log`` argument for more details. epoch_event_writer: customized callable TensorBoard writer for epoch level. Must accept parameter "engine" and "summary_writer", use default event writer if None. epoch_interval: the epoch interval at which the epoch_event_writer is called. Defaults to 1. + ``epoch_interval`` must be 1 if ``epoch_log`` is callable. iteration_event_writer: customized callable TensorBoard writer for iteration level. Must accept parameter "engine" and "summary_writer", use default event writer if None. iteration_interval: the iteration interval at which the iteration_event_writer is called. Defaults to 1. + ``iteration_interval`` must be 1 if ``iteration_log`` is callable. output_transform: a callable that is used to transform the ``ignite.engine.state.output`` into a scalar to plot, or a dictionary of {key: scalar}. In the latter case, the output string will be formatted as key: value. @@ -131,6 +138,12 @@ def __init__( when epoch completed. tag_name: when iteration output is a scalar, tag_name is used to plot, defaults to ``'Loss'``. """ + if callable(iteration_log) and iteration_interval > 1: + raise ValueError("If iteration_log is callable, then iteration_interval should be 1") + + if callable(epoch_log) and epoch_interval > 1: + raise ValueError("If epoch_log is callable, then epoch_interval should be 1") + super().__init__(summary_writer=summary_writer, log_dir=log_dir) self.iteration_log = iteration_log self.epoch_log = epoch_log @@ -152,11 +165,19 @@ def attach(self, engine: Engine) -> None: """ if self.iteration_log and not engine.has_event_handler(self.iteration_completed, Events.ITERATION_COMPLETED): - engine.add_event_handler( - Events.ITERATION_COMPLETED(every=self.iteration_interval), self.iteration_completed - ) + event = Events.ITERATION_COMPLETED + if callable(self.iteration_log): # substitute event with new one using filter callable + event = event(event_filter=self.iteration_log) + elif self.iteration_interval > 1: + event = event(every=self.iteration_interval) + engine.add_event_handler(event, 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(every=self.epoch_interval), self.epoch_completed) + event = Events.EPOCH_COMPLETED + if callable(self.epoch_log): # substitute event with new one using filter callable + event = event(event_filter=self.epoch_log) + elif self.epoch_log > 1: + event = event(every=self.epoch_interval) + engine.add_event_handler(event, self.epoch_completed) def epoch_completed(self, engine: Engine) -> None: """ diff --git a/tests/test_handler_tb_stats.py b/tests/test_handler_tb_stats.py index b135dee86e..745e0b60f7 100644 --- a/tests/test_handler_tb_stats.py +++ b/tests/test_handler_tb_stats.py @@ -14,8 +14,10 @@ import glob import tempfile import unittest +from unittest.mock import MagicMock from ignite.engine import Engine, Events +from parameterized import parameterized from monai.handlers import TensorBoardStatsHandler from monai.utils import optional_import @@ -23,8 +25,24 @@ SummaryWriter, has_tb = optional_import("torch.utils.tensorboard", name="SummaryWriter") +def get_event_filter(e): + def event_filter(_, event): + if event in e: + return True + return False + + return event_filter + + @unittest.skipUnless(has_tb, "Requires SummaryWriter installation") class TestHandlerTBStats(unittest.TestCase): + def test_args_validation(self): + with self.assertRaisesRegex(ValueError, expected_regex="iteration_interval should be 1"): + TensorBoardStatsHandler(log_dir=".", iteration_log=get_event_filter([1, 2]), iteration_interval=2) + + with self.assertRaisesRegex(ValueError, expected_regex="epoch_interval should be 1"): + TensorBoardStatsHandler(log_dir=".", epoch_log=get_event_filter([1, 2]), epoch_interval=2) + def test_metrics_print(self): with tempfile.TemporaryDirectory() as tempdir: # set up engine @@ -47,6 +65,35 @@ def _update_metric(engine): # check logging output self.assertTrue(len(glob.glob(tempdir)) > 0) + @parameterized.expand([[True], [get_event_filter([1, 2])]]) + def test_metrics_print_mock(self, epoch_log): + with tempfile.TemporaryDirectory() as tempdir: + # set up engine + def _train_func(engine, batch): + return [batch + 1.0] + + engine = Engine(_train_func) + + # set up dummy metric + @engine.on(Events.EPOCH_COMPLETED) + def _update_metric(engine): + current_metric = engine.state.metrics.get("acc", 0.1) + engine.state.metrics["acc"] = current_metric + 0.1 + + # set up testing handler + stats_handler = TensorBoardStatsHandler(log_dir=tempdir, iteration_log=False, epoch_log=epoch_log) + stats_handler._default_epoch_writer = MagicMock() + stats_handler.attach(engine) + + max_epochs = 4 + engine.run(range(3), max_epochs=max_epochs) + stats_handler.close() + # check logging output + if epoch_log is True: + self.assertEqual(stats_handler._default_epoch_writer.call_count, max_epochs) + else: + self.assertEqual(stats_handler._default_epoch_writer.call_count, 2) # 2 = len([1, 2]) from event_filter + def test_metrics_writer(self): with tempfile.TemporaryDirectory() as tempdir: # set up engine @@ -78,6 +125,47 @@ def _update_metric(engine): # check logging output self.assertTrue(len(glob.glob(tempdir)) > 0) + @parameterized.expand([[True], [get_event_filter([1, 3])]]) + def test_metrics_writer_mock(self, iteration_log): + with tempfile.TemporaryDirectory() as tempdir: + # set up engine + def _train_func(engine, batch): + return [batch + 1.0] + + engine = Engine(_train_func) + + # set up dummy metric + @engine.on(Events.EPOCH_COMPLETED) + def _update_metric(engine): + current_metric = engine.state.metrics.get("acc", 0.1) + engine.state.metrics["acc"] = current_metric + 0.1 + engine.state.test = current_metric + + # set up testing handler + writer = SummaryWriter(log_dir=tempdir) + stats_handler = TensorBoardStatsHandler( + summary_writer=writer, + iteration_log=iteration_log, + epoch_log=False, + output_transform=lambda x: {"loss": x[0] * 2.0}, + global_epoch_transform=lambda x: x * 3.0, + state_attributes=["test"], + ) + stats_handler._default_iteration_writer = MagicMock() + stats_handler.attach(engine) + + num_iters = 3 + max_epochs = 2 + engine.run(range(num_iters), max_epochs=max_epochs) + writer.close() + + if iteration_log is True: + self.assertEqual(stats_handler._default_iteration_writer.call_count, num_iters * max_epochs) + else: + self.assertEqual( + stats_handler._default_iteration_writer.call_count, 2 + ) # 2 = len([1, 3]) from event_filter + if __name__ == "__main__": unittest.main() From efed142dddf664da9926e5e3b863ce6f0a0e9d10 Mon Sep 17 00:00:00 2001 From: vfdev-5 Date: Fri, 10 Feb 2023 12:19:53 +0100 Subject: [PATCH 2/5] Updated mlflow stats handler iter/epoch args --- monai/handlers/mlflow_handler.py | 19 +++++-- tests/test_handler_mlflow.py | 90 ++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/monai/handlers/mlflow_handler.py b/monai/handlers/mlflow_handler.py index d8871d2d87..e49a2e967e 100644 --- a/monai/handlers/mlflow_handler.py +++ b/monai/handlers/mlflow_handler.py @@ -60,7 +60,12 @@ class MLFlowHandler: to log data to a directory. The URI defaults to path `mlruns`. for more details: https://mlflow.org/docs/latest/python_api/mlflow.html#mlflow.set_tracking_uri. iteration_log: whether to log data to MLFlow when iteration completed, default to `True`. + ``iteration_log`` can be also a function and it will be interpreted as an event filter + (see https://pytorch.org/ignite/generated/ignite.engine.events.Events.html for details). + Event filter function accepts as input engine and event value (iteration) and should return True/False. epoch_log: whether to log data to MLFlow when epoch completed, default to `True`. + ``epoch_log`` can be also a function and it will be interpreted as an event filter. + See ``iteration_log`` argument for more details. epoch_logger: customized callable logger for epoch level logging with MLFlow. Must accept parameter "engine", use default logger if None. iteration_logger: customized callable logger for iteration level logging with MLFlow. @@ -98,8 +103,8 @@ class MLFlowHandler: def __init__( self, tracking_uri: str | None = None, - iteration_log: bool = True, - epoch_log: bool = True, + iteration_log: bool | Callable[[Engine, int], bool] = True, + epoch_log: bool | Callable[[Engine, int], bool] = True, epoch_logger: Callable[[Engine], Any] | None = None, iteration_logger: Callable[[Engine], Any] | None = None, output_transform: Callable = lambda x: x[0], @@ -159,9 +164,15 @@ def attach(self, engine: Engine) -> None: if not engine.has_event_handler(self.start, Events.STARTED): engine.add_event_handler(Events.STARTED, self.start) if self.iteration_log and not engine.has_event_handler(self.iteration_completed, Events.ITERATION_COMPLETED): - engine.add_event_handler(Events.ITERATION_COMPLETED, self.iteration_completed) + event = Events.ITERATION_COMPLETED + if callable(self.iteration_log): # substitute event with new one using filter callable + event = event(event_filter=self.iteration_log) + engine.add_event_handler(event, 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) + event = Events.EPOCH_COMPLETED + if callable(self.epoch_log): # substitute event with new one using filter callable + event = event(event_filter=self.epoch_log) + engine.add_event_handler(event, self.epoch_completed) if not engine.has_event_handler(self.complete, Events.COMPLETED): engine.add_event_handler(Events.COMPLETED, self.complete) if self.close_on_complete and (not engine.has_event_handler(self.close, Events.COMPLETED)): diff --git a/tests/test_handler_mlflow.py b/tests/test_handler_mlflow.py index 99180860a7..d9474a9a72 100644 --- a/tests/test_handler_mlflow.py +++ b/tests/test_handler_mlflow.py @@ -17,14 +17,25 @@ import tempfile import unittest from concurrent.futures import ThreadPoolExecutor +from unittest.mock import MagicMock import numpy as np from ignite.engine import Engine, Events +from parameterized import parameterized from monai.handlers import MLFlowHandler from monai.utils import path_to_uri +def get_event_filter(e): + def event_filter(_, event): + if event in e: + return True + return False + + return event_filter + + def dummy_train(tracking_folder): tempdir = tempfile.mkdtemp() @@ -95,6 +106,85 @@ def _update_metric(engine): # check logging output self.assertTrue(len(glob.glob(test_path)) > 0) + @parameterized.expand([[True], [get_event_filter([1, 2])]]) + def test_metrics_track_mock(self, epoch_log): + experiment_param = {"backbone": "efficientnet_b0"} + with tempfile.TemporaryDirectory() as tempdir: + # set up engine + def _train_func(engine, batch): + return [batch + 1.0] + + engine = Engine(_train_func) + + # set up dummy metric + @engine.on(Events.EPOCH_COMPLETED) + def _update_metric(engine): + current_metric = engine.state.metrics.get("acc", 0.1) + engine.state.metrics["acc"] = current_metric + 0.1 + engine.state.test = current_metric + + # set up testing handler + test_path = os.path.join(tempdir, "mlflow_test") + handler = MLFlowHandler( + iteration_log=False, + epoch_log=epoch_log, + tracking_uri=path_to_uri(test_path), + state_attributes=["test"], + experiment_param=experiment_param, + close_on_complete=True, + ) + handler._default_epoch_log = MagicMock() + handler.attach(engine) + + max_epochs = 4 + engine.run(range(3), max_epochs=max_epochs) + handler.close() + # check logging output + if epoch_log is True: + self.assertEqual(handler._default_epoch_log.call_count, max_epochs) + else: + self.assertEqual(handler._default_epoch_log.call_count, 2) # 2 = len([1, 2]) from event_filter + + @parameterized.expand([[True], [get_event_filter([1, 3])]]) + def test_metrics_track_iters_mock(self, iteration_log): + experiment_param = {"backbone": "efficientnet_b0"} + with tempfile.TemporaryDirectory() as tempdir: + # set up engine + def _train_func(engine, batch): + return [batch + 1.0] + + engine = Engine(_train_func) + + # set up dummy metric + @engine.on(Events.EPOCH_COMPLETED) + def _update_metric(engine): + current_metric = engine.state.metrics.get("acc", 0.1) + engine.state.metrics["acc"] = current_metric + 0.1 + engine.state.test = current_metric + + # set up testing handler + test_path = os.path.join(tempdir, "mlflow_test") + handler = MLFlowHandler( + iteration_log=iteration_log, + epoch_log=False, + tracking_uri=path_to_uri(test_path), + state_attributes=["test"], + experiment_param=experiment_param, + close_on_complete=True, + ) + handler._default_iteration_log = MagicMock() + handler.attach(engine) + + num_iters = 3 + max_epochs = 2 + engine.run(range(num_iters), max_epochs=max_epochs) + handler.close() + # check logging output + if iteration_log is True: + self.assertEqual(handler._default_iteration_log.call_count, num_iters * max_epochs) + else: + self.assertEqual(handler._default_iteration_log.call_count, 2) # 2 = len([1, 3]) from event_filter + def test_multi_thread(self): test_uri_list = ["monai_mlflow_test1", "monai_mlflow_test2"] with ThreadPoolExecutor(2, "Training") as executor: From f1b1810fb76fab92e7c060b1cccad8ce48fd8b4d Mon Sep 17 00:00:00 2001 From: vfdev-5 Date: Mon, 20 Feb 2023 11:46:46 +0100 Subject: [PATCH 3/5] Deprecated TB args --- monai/handlers/tensorboard_handlers.py | 4 +++- tests/test_handler_tb_stats.py | 9 +++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/monai/handlers/tensorboard_handlers.py b/monai/handlers/tensorboard_handlers.py index 1cff275866..3eb2fe1280 100644 --- a/monai/handlers/tensorboard_handlers.py +++ b/monai/handlers/tensorboard_handlers.py @@ -19,7 +19,7 @@ import torch from monai.config import IgniteInfo -from monai.utils import is_scalar, min_version, optional_import +from monai.utils import deprecated_arg, is_scalar, min_version, optional_import from monai.visualize import plot_2d_or_3d_image Events, _ = optional_import("ignite.engine", IgniteInfo.OPT_IMPORT_VERSION, min_version, "Events") @@ -87,6 +87,8 @@ class TensorBoardStatsHandler(TensorBoardHandler): """ + @deprecated_arg("epoch_interval", since="1.1", removed="1.3") + @deprecated_arg("iteration_interval", since="1.1", removed="1.3") def __init__( self, summary_writer: SummaryWriter | SummaryWriterX | None = None, diff --git a/tests/test_handler_tb_stats.py b/tests/test_handler_tb_stats.py index 745e0b60f7..f4ccfc9780 100644 --- a/tests/test_handler_tb_stats.py +++ b/tests/test_handler_tb_stats.py @@ -37,11 +37,12 @@ def event_filter(_, event): @unittest.skipUnless(has_tb, "Requires SummaryWriter installation") class TestHandlerTBStats(unittest.TestCase): def test_args_validation(self): - with self.assertRaisesRegex(ValueError, expected_regex="iteration_interval should be 1"): - TensorBoardStatsHandler(log_dir=".", iteration_log=get_event_filter([1, 2]), iteration_interval=2) + with self.assertWarns(FutureWarning): + with self.assertRaisesRegex(ValueError, expected_regex="iteration_interval should be 1"): + TensorBoardStatsHandler(log_dir=".", iteration_log=get_event_filter([1, 2]), iteration_interval=2) - with self.assertRaisesRegex(ValueError, expected_regex="epoch_interval should be 1"): - TensorBoardStatsHandler(log_dir=".", epoch_log=get_event_filter([1, 2]), epoch_interval=2) + with self.assertRaisesRegex(ValueError, expected_regex="epoch_interval should be 1"): + TensorBoardStatsHandler(log_dir=".", epoch_log=get_event_filter([1, 2]), epoch_interval=2) def test_metrics_print(self): with tempfile.TemporaryDirectory() as tempdir: From 36affc79f7a1a09940e901a23f4992717ac8f87e Mon Sep 17 00:00:00 2001 From: Wenqi Li Date: Mon, 20 Feb 2023 15:15:16 +0000 Subject: [PATCH 4/5] fixes deprected version Signed-off-by: Wenqi Li --- monai/utils/deprecate_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monai/utils/deprecate_utils.py b/monai/utils/deprecate_utils.py index ed58111504..d4f239cd23 100644 --- a/monai/utils/deprecate_utils.py +++ b/monai/utils/deprecate_utils.py @@ -174,7 +174,7 @@ def deprecated_arg( else: # compare the numbers is_deprecated = since is not None and version_leq(since, version_val) - is_removed = removed is not None and version_leq(removed, version_val) + is_removed = removed is not None and version_val != f"{sys.maxsize}" and version_leq(removed, version_val) def _decorator(func): argname = f"{func.__module__} {func.__qualname__}:{name}" @@ -284,7 +284,7 @@ def deprecated_arg_default( else: # compare the numbers is_deprecated = since is not None and version_leq(since, version_val) - is_replaced = replaced is not None and version_leq(replaced, version_val) + is_replaced = replaced is not None and version_val != f"{sys.maxsize}" and version_leq(replaced, version_val) def _decorator(func): argname = f"{func.__module__} {func.__qualname__}:{name}" From d33d403b79010ff277257134387f3896b4fe99ce Mon Sep 17 00:00:00 2001 From: Wenqi Li Date: Mon, 20 Feb 2023 15:21:04 +0000 Subject: [PATCH 5/5] testing deprecated errors Signed-off-by: Wenqi Li --- tests/test_deprecated.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_deprecated.py b/tests/test_deprecated.py index 1aac97e804..5d511f3821 100644 --- a/tests/test_deprecated.py +++ b/tests/test_deprecated.py @@ -234,7 +234,7 @@ def test_arg_except2_unknown(self): def afoo4(a, b=None): pass - self.assertRaises(DeprecatedError, lambda: afoo4(1, b=2)) + afoo4(1, b=2) def test_arg_except3_unknown(self): """ @@ -246,8 +246,8 @@ def test_arg_except3_unknown(self): def afoo4(a, b=None, **kwargs): pass - self.assertRaises(DeprecatedError, lambda: afoo4(1, b=2)) - self.assertRaises(DeprecatedError, lambda: afoo4(1, b=2, c=3)) + afoo4(1, b=2) + afoo4(1, b=2, c=3) def test_replacement_arg(self): """