From 46263ef15f8aec5210cb722e10a312c25c77fb81 Mon Sep 17 00:00:00 2001 From: eracah Date: Thu, 28 Sep 2023 14:53:36 -0700 Subject: [PATCH 1/2] Add syntactic sugar for loggers --- composer/loggers/logger.py | 121 +++++++++++++++++++++++++++++++++++ tests/loggers/test_logger.py | 34 +++++++++- 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/composer/loggers/logger.py b/composer/loggers/logger.py index e1567dfbb3..1f27ea71ef 100644 --- a/composer/loggers/logger.py +++ b/composer/loggers/logger.py @@ -147,6 +147,127 @@ def has_file_upload_destination(self) -> bool: return True return False + def _get_logger_destination(self, logger_type) -> Optional[Union[LoggerDestination, Sequence[LoggerDestination]]]: + """Returns instances of the specified logger destination, if it/they exist.""" + # Check every time just in case user appended to destinations list later. + logger_destinations = [destination for destination in self.destinations if isinstance(destination, logger_type)] + if len(logger_destinations) == 1: + return logger_destinations[0] + elif len(logger_destinations) > 1: + return logger_destinations + else: # length is 0 + return None + + @property + def wandb(self): + """Returns instances of the :class:`~.WandBLogger` destination, if it/they exist. + + Returns: + Optional[Union[WandBLogger, Sequence[WandBLogger]]]: The WandBLogger instance(s), if it/they exist. + """ + from composer.loggers.wandb_logger import WandBLogger + return self._get_logger_destination(WandBLogger) + + @property + def mlflow(self): + """Returns instances of the :class:`~.MLFlowLogger` destination, if it/they exist. + + Returns: + Optional[Union[MLFlowLogger, Sequence[MLFlowLogger]]]: The MLFlowLogger instance(s), if it/they exist. + """ + from composer.loggers.mlflow_logger import MLFlowLogger + return self._get_logger_destination(MLFlowLogger) + + @property + def remote_uploader_downloader(self): + """Returns instances of the :class:`~.RemoteUploaderDownloader` destination, if it/they exist. + + Returns: + Optional[Union[RemoteUploaderDownloader, Sequence[RemoteUploaderDownloader]]]: The RemoteUploaderDownloader instance(s), if it/they exist. + """ + from composer.loggers.remote_uploader_downloader import RemoteUploaderDownloader + return self._get_logger_destination(RemoteUploaderDownloader) + + @property + def mosaicml(self): + """Returns instances of the :class:`~.MosaicMLLogger` destination, if it/they exist. + + Returns: + Optional[Union[MosaicMLLogger, Sequence[MosaicMLLogger]]]: The MosaicMLLogger instance(s), if it/they exist. + """ + from composer.loggers.mosaicml_logger import MosaicMLLogger + return self._get_logger_destination(MosaicMLLogger) + + @property + def slack(self): + """Returns instances of the :class:`~.SlackLogger` destination, if it/they exist. + + Returns: + Optional[Union[SlackLogger, Sequence[SlackLogger]]]: The SlackLogger instance(s), if it/they exist. + """ + from composer.loggers.slack_logger import SlackLogger + return self._get_logger_destination(SlackLogger) + + @property + def console(self): + """Returns instances of the :class:`~.ConsoleLogger` destination, if it/they exist. + + Returns: + Optional[Union[ConsoleLogger, Sequence[ConsoleLogger]]]: The ConsoleLogger instance(s), if it/they exist. + """ + from composer.loggers.console_logger import ConsoleLogger + return self._get_logger_destination(ConsoleLogger) + + @property + def file(self): + """Returns instances of the :class:`~.FileLogger` destination, if it/they exist. + + Returns: + Optional[Union[FileLogger, Sequence[FileLogger]]]: The FileLogger instance(s), if it/they exist. + """ + from composer.loggers.file_logger import FileLogger + return self._get_logger_destination(FileLogger) + + @property + def in_memory(self): + """Returns instances of the :class:`~.InMemoryLogger` destination, if it/they exist. + + Returns: + Optional[Union[InMemoryLogger, Sequence[InMemoryLogger]]]: The InMemoryLogger instance(s), if it/they exist. + """ + from composer.loggers.in_memory_logger import InMemoryLogger + return self._get_logger_destination(InMemoryLogger) + + @property + def progress_bar(self): + """Returns instances of the :class:`~.ProgressBarLogger` destination, if it/they exist. + + Returns: + Optional[Union[ProgressBarLogger, Sequence[ProgressBarLogger]]]: The ProgressBarLogger instance(s), if it/they exist. + """ + from composer.loggers.progress_bar_logger import ProgressBarLogger + return self._get_logger_destination(ProgressBarLogger) + + @property + def tensorboard(self): + """Returns instances of the :class:`~.TensorboardLogger` destination, if it/they exist. + + Returns: + Optional[Union[TensorboardLogger, Sequence[TensorboardLogger]]]: The TensorboardLogger instance(s), if it/they exist. + """ + from composer.loggers.tensorboard_logger import TensorboardLogger + return self._get_logger_destination(TensorboardLogger) + + @property + def cometml(self): + """Returns instances of the :class:`~.CometMLLogger` destination, if it/they exist. + + Returns: + Optional[Union[CometMLLogger, Sequence[CometMLLogger]]]: The CometMLLogger instance(s), if it/they exist. + """ + from composer.loggers.cometml_logger import CometMLLogger + return self._get_logger_destination(CometMLLogger) + def format_log_data_value(data: Any) -> str: """Recursively formats a given log data value into a string. diff --git a/tests/loggers/test_logger.py b/tests/loggers/test_logger.py index 9a8e608fa9..fe82a3346d 100644 --- a/tests/loggers/test_logger.py +++ b/tests/loggers/test_logger.py @@ -2,9 +2,41 @@ # SPDX-License-Identifier: Apache-2.0 import pathlib +from typing import List + +import pytest from composer.core.state import State -from composer.loggers import Logger, LoggerDestination +from composer.loggers import (ConsoleLogger, FileLogger, InMemoryLogger, Logger, LoggerDestination, MLFlowLogger, + MosaicMLLogger, ProgressBarLogger, RemoteUploaderDownloader, SlackLogger, + TensorboardLogger, WandBLogger) + + +@pytest.mark.parametrize('num_dest_instances_per_type', [0, 1, 2]) +def test_logger_properties(dummy_state: State, num_dest_instances_per_type: int): + logger_destination_classes = (ConsoleLogger, FileLogger, InMemoryLogger, MLFlowLogger, MosaicMLLogger, + ProgressBarLogger, SlackLogger, TensorboardLogger, WandBLogger) + destinations = [] + + for _ in range(num_dest_instances_per_type): + for logger_destination_type_cls in logger_destination_classes: + logger_dest = logger_destination_type_cls() + destinations.append(logger_dest) + destinations.append(RemoteUploaderDownloader(bucket_uri='foo')) + logger = Logger(state=dummy_state, destinations=destinations) + for logger_destination_property_name in [ + 'wandb', 'mlflow', 'mosaicml', 'tensorboard', 'slack', 'console', 'progress_bar', 'file', 'in_memory', + 'remote_uploader_downloader' + ]: + assert hasattr(logger, logger_destination_property_name) + if num_dest_instances_per_type == 0: + assert getattr(logger, logger_destination_property_name) is None + else: + assert getattr(logger, logger_destination_property_name) is not None + if num_dest_instances_per_type == 1: + assert not isinstance(getattr(logger, logger_destination_property_name), List) + else: + assert isinstance(getattr(logger, logger_destination_property_name), List) def test_logger_file_upload(dummy_state: State): From 63cb9428e8acc792f47b4ed4f53c609df00270fb Mon Sep 17 00:00:00 2001 From: eracah Date: Tue, 17 Oct 2023 14:07:11 -0700 Subject: [PATCH 2/2] fix typing --- composer/loggers/logger.py | 63 +++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/composer/loggers/logger.py b/composer/loggers/logger.py index 1f27ea71ef..47a54a2a39 100644 --- a/composer/loggers/logger.py +++ b/composer/loggers/logger.py @@ -18,6 +18,9 @@ if TYPE_CHECKING: from composer.core import State + from composer.loggers import (CometMLLogger, ConsoleLogger, FileLogger, InMemoryLogger, MLFlowLogger, + MosaicMLLogger, ProgressBarLogger, RemoteUploaderDownloader, SlackLogger, + TensorboardLogger, WandBLogger) from composer.loggers.logger_destination import LoggerDestination __all__ = ['LoggerDestination', 'Logger', 'format_log_data_value'] @@ -147,126 +150,116 @@ def has_file_upload_destination(self) -> bool: return True return False - def _get_logger_destination(self, logger_type) -> Optional[Union[LoggerDestination, Sequence[LoggerDestination]]]: - """Returns instances of the specified logger destination, if it/they exist.""" - # Check every time just in case user appended to destinations list later. - logger_destinations = [destination for destination in self.destinations if isinstance(destination, logger_type)] - if len(logger_destinations) == 1: - return logger_destinations[0] - elif len(logger_destinations) > 1: - return logger_destinations - else: # length is 0 - return None - @property - def wandb(self): + def wandb(self) -> Optional[Sequence['WandBLogger']]: """Returns instances of the :class:`~.WandBLogger` destination, if it/they exist. Returns: - Optional[Union[WandBLogger, Sequence[WandBLogger]]]: The WandBLogger instance(s), if it/they exist. + Optional[Sequence[WandBLogger]]: The WandBLogger instance(s), if it/they exist. + """ from composer.loggers.wandb_logger import WandBLogger - return self._get_logger_destination(WandBLogger) + return [destination for destination in self.destinations if isinstance(destination, WandBLogger)] @property - def mlflow(self): + def mlflow(self) -> Optional[Sequence['MLFlowLogger']]: """Returns instances of the :class:`~.MLFlowLogger` destination, if it/they exist. Returns: - Optional[Union[MLFlowLogger, Sequence[MLFlowLogger]]]: The MLFlowLogger instance(s), if it/they exist. + Optional[Sequence[MLFlowLogger]]: The MLFlowLogger instance(s), if it/they exist. """ from composer.loggers.mlflow_logger import MLFlowLogger - return self._get_logger_destination(MLFlowLogger) + return [destination for destination in self.destinations if isinstance(destination, MLFlowLogger)] @property - def remote_uploader_downloader(self): + def remote_uploader_downloader(self) -> Optional[Sequence['RemoteUploaderDownloader']]: """Returns instances of the :class:`~.RemoteUploaderDownloader` destination, if it/they exist. Returns: Optional[Union[RemoteUploaderDownloader, Sequence[RemoteUploaderDownloader]]]: The RemoteUploaderDownloader instance(s), if it/they exist. """ from composer.loggers.remote_uploader_downloader import RemoteUploaderDownloader - return self._get_logger_destination(RemoteUploaderDownloader) + return [destination for destination in self.destinations if isinstance(destination, RemoteUploaderDownloader)] @property - def mosaicml(self): + def mosaicml(self) -> Optional[Sequence['MosaicMLLogger']]: """Returns instances of the :class:`~.MosaicMLLogger` destination, if it/they exist. Returns: Optional[Union[MosaicMLLogger, Sequence[MosaicMLLogger]]]: The MosaicMLLogger instance(s), if it/they exist. """ from composer.loggers.mosaicml_logger import MosaicMLLogger - return self._get_logger_destination(MosaicMLLogger) + return [destination for destination in self.destinations if isinstance(destination, MosaicMLLogger)] @property - def slack(self): + def slack(self) -> Optional[Sequence['SlackLogger']]: """Returns instances of the :class:`~.SlackLogger` destination, if it/they exist. Returns: Optional[Union[SlackLogger, Sequence[SlackLogger]]]: The SlackLogger instance(s), if it/they exist. """ from composer.loggers.slack_logger import SlackLogger - return self._get_logger_destination(SlackLogger) + return [destination for destination in self.destinations if isinstance(destination, SlackLogger)] @property - def console(self): + def console(self) -> Optional[Sequence['ConsoleLogger']]: """Returns instances of the :class:`~.ConsoleLogger` destination, if it/they exist. Returns: Optional[Union[ConsoleLogger, Sequence[ConsoleLogger]]]: The ConsoleLogger instance(s), if it/they exist. """ from composer.loggers.console_logger import ConsoleLogger - return self._get_logger_destination(ConsoleLogger) + return [destination for destination in self.destinations if isinstance(destination, ConsoleLogger)] @property - def file(self): + def file(self) -> Optional[Sequence['FileLogger']]: """Returns instances of the :class:`~.FileLogger` destination, if it/they exist. Returns: Optional[Union[FileLogger, Sequence[FileLogger]]]: The FileLogger instance(s), if it/they exist. """ from composer.loggers.file_logger import FileLogger - return self._get_logger_destination(FileLogger) + return [destination for destination in self.destinations if isinstance(destination, FileLogger)] @property - def in_memory(self): + def in_memory(self) -> Optional[Sequence['InMemoryLogger']]: """Returns instances of the :class:`~.InMemoryLogger` destination, if it/they exist. Returns: Optional[Union[InMemoryLogger, Sequence[InMemoryLogger]]]: The InMemoryLogger instance(s), if it/they exist. """ from composer.loggers.in_memory_logger import InMemoryLogger - return self._get_logger_destination(InMemoryLogger) + return [destination for destination in self.destinations if isinstance(destination, InMemoryLogger)] @property - def progress_bar(self): + def progress_bar(self) -> Optional[Sequence['ProgressBarLogger']]: """Returns instances of the :class:`~.ProgressBarLogger` destination, if it/they exist. Returns: Optional[Union[ProgressBarLogger, Sequence[ProgressBarLogger]]]: The ProgressBarLogger instance(s), if it/they exist. """ from composer.loggers.progress_bar_logger import ProgressBarLogger - return self._get_logger_destination(ProgressBarLogger) + return [destination for destination in self.destinations if isinstance(destination, ProgressBarLogger)] @property - def tensorboard(self): + def tensorboard(self) -> Optional[Sequence['TensorboardLogger']]: """Returns instances of the :class:`~.TensorboardLogger` destination, if it/they exist. Returns: Optional[Union[TensorboardLogger, Sequence[TensorboardLogger]]]: The TensorboardLogger instance(s), if it/they exist. """ from composer.loggers.tensorboard_logger import TensorboardLogger - return self._get_logger_destination(TensorboardLogger) + return [destination for destination in self.destinations if isinstance(destination, TensorboardLogger)] @property - def cometml(self): + def cometml(self) -> Optional[Sequence['CometMLLogger']]: """Returns instances of the :class:`~.CometMLLogger` destination, if it/they exist. Returns: Optional[Union[CometMLLogger, Sequence[CometMLLogger]]]: The CometMLLogger instance(s), if it/they exist. """ from composer.loggers.cometml_logger import CometMLLogger - return self._get_logger_destination(CometMLLogger) + return [destination for destination in self.destinations if isinstance(destination, CometMLLogger)] def format_log_data_value(data: Any) -> str: