From 41a49a1f85cbb0ee6d9effc3e609ebc128c5e433 Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Wed, 31 Aug 2022 14:30:46 -0700 Subject: [PATCH 01/12] add comet_ml installation --- setup.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/setup.py b/setup.py index 5a22e264e4..b6a69cd1bf 100644 --- a/setup.py +++ b/setup.py @@ -144,6 +144,10 @@ def package_files(prefix: str, directory: str, extension: str): 'wandb>=0.13.2,<0.14', ] +extra_deps['comet_ml'] = [ + 'comet_ml>=3.31.12' +] + extra_deps['tensorboard'] = [ 'tensorboard>=2.9.1,<3.0.0', 'tensorflow-io>=0.26.0,<0.27', From 5955c0b969d2381b8d64bccea50552e81da65a6d Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Wed, 31 Aug 2022 17:49:24 -0700 Subject: [PATCH 02/12] Add CometMLLogger --- composer/loggers/cometml_logger.py | 101 +++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 composer/loggers/cometml_logger.py diff --git a/composer/loggers/cometml_logger.py b/composer/loggers/cometml_logger.py new file mode 100644 index 0000000000..149210682d --- /dev/null +++ b/composer/loggers/cometml_logger.py @@ -0,0 +1,101 @@ +# Copyright 2022 MosaicML Composer authors +# SPDX-License-Identifier: Apache-2.0 + +"""Log to `Comet `_.""" + +from __future__ import annotations + +from typing import Any, Dict, Optional +from composer.core.state import State +from composer.loggers.logger import Logger +from composer.loggers.logger_destination import LoggerDestination +from composer.utils import dist +from composer.utils.import_helpers import MissingConditionalImportError + +__all__ = ['CometMLLogger'] + + +class CometMLLogger(LoggerDestination): + """Log to `Comet `_. + + Args: + workspace (str, optional): The name of the workspace which contains the project + you want to attach your experiment to. If nothing specified will default to your + default workspace as configured at `www.comet.com `. + project_name (str, optional): The name of the project to categorize your experiment in. + A new project with this name will be created under the Comet workspace if one + with this name does not exist. If no project name specified, the experiment will go + under 'Uncategorized Experiments'. + log_code (bool): Whether to log your code in your experiment (default: ``False``). + log_graph (bool): Whether to log your computational graph in your experiment + (default: ``False``). + name (str, optional): The name of your experiment. If not specified, it will be set + to :attr:`.State.run_name`. + rank_zero_only (bool, optional): Whether to log only on the rank-zero process. + (default: ``False``). + init_kwargs (Dict[str, Any], optional): Any additional init kwargs + ``wandb.init`` (see + `WandB do`cumentation `_). + """ + + def __init__( + self, + workspace: Optional[str] = None, + project_name: Optional[str] = None, + log_code: bool = False, + log_graph: bool = False, + name: Optional[str] = None, + rank_zero_only: bool = True, + init_kwargs: Optional[Dict[str, Any]] = None, + ) -> None: + try: + import comet_ml + except ImportError as e: + raise MissingConditionalImportError(extra_deps_group='comet_ml', + conda_package='comet_ml', + conda_channel='conda-forge') from e + + self._enabled = (not rank_zero_only) or dist.get_global_rank() == 0 + + if init_kwargs is None: + init_kwargs = {} + + if workspace is not None: + init_kwargs['workspace'] = workspace + + if project_name is not None: + init_kwargs['project_name'] = project_name + + init_kwargs['log_code'] = log_code + init_kwargs['log_graph'] = log_graph + + + self.name = name + self._rank_zero_only = rank_zero_only + self._init_kwargs = init_kwargs + self.experiment = Optional[comet_ml.Experiment] = None + + def init(self, state: State, logger: Logger) -> None: + import comet_ml + del logger # unused + + # Use the logger run name if the name is not set. + if self.name is None: + self.name = state.run_name + + # Adjust name and group based on `rank_zero_only`. + if not self._rank_zero_only: + self.name += f'-rank{dist.get_global_rank()}' + + if self._enabled: + self.experiment = comet_ml.Experiment(**self._init_kwargs) + self.experiment.set_name(self.name) + + def log_metrics(self, metrics: Dict[str, Any], step: Optional[int] = None) -> None: + if self._enabled: + self.experiment.log_metrics(dic=metrics, step=step) + + def log_hyperparameters(self, hyperparameters: Dict[str, Any]): + if self._enabled: + self.experiment.log_parameters(hyperparameters) + From 8fa752cdd9f1c7ef9a853c0e6bca5c18b8e490f1 Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Wed, 31 Aug 2022 18:58:47 -0700 Subject: [PATCH 03/12] Add in rest of glue code for CometMlLogger --- composer/loggers/__init__.py | 2 + composer/loggers/cometml_logger.py | 41 ++++++++++---------- composer/loggers/logger_hparams_registry.py | 2 + docs/source/getting_started/installation.rst | 1 + docs/source/notes/run_name.md | 10 ++--- docs/source/trainer/logging.rst | 1 + examples/run_composer_trainer.py | 10 +---- setup.py | 4 +- tests/callbacks/callback_settings.py | 10 ++++- 9 files changed, 43 insertions(+), 38 deletions(-) diff --git a/composer/loggers/__init__.py b/composer/loggers/__init__.py index ab94d5021d..84e696c8b0 100644 --- a/composer/loggers/__init__.py +++ b/composer/loggers/__init__.py @@ -12,6 +12,7 @@ define a custom logger and use it when training. """ +from composer.loggers.cometml_logger import CometMLLogger from composer.loggers.file_logger import FileLogger from composer.loggers.in_memory_logger import InMemoryLogger from composer.loggers.logger import Logger, LogLevel @@ -32,4 +33,5 @@ 'WandBLogger', 'ObjectStoreLogger', 'TensorboardLogger', + 'CometMLLogger', ] diff --git a/composer/loggers/cometml_logger.py b/composer/loggers/cometml_logger.py index 149210682d..59ce0bb1fe 100644 --- a/composer/loggers/cometml_logger.py +++ b/composer/loggers/cometml_logger.py @@ -6,6 +6,7 @@ from __future__ import annotations from typing import Any, Dict, Optional + from composer.core.state import State from composer.loggers.logger import Logger from composer.loggers.logger_destination import LoggerDestination @@ -21,11 +22,11 @@ class CometMLLogger(LoggerDestination): Args: workspace (str, optional): The name of the workspace which contains the project you want to attach your experiment to. If nothing specified will default to your - default workspace as configured at `www.comet.com `. - project_name (str, optional): The name of the project to categorize your experiment in. + default workspace as configured in your comet account settings. + project_name (str, optional): The name of the project to categorize your experiment in. A new project with this name will be created under the Comet workspace if one - with this name does not exist. If no project name specified, the experiment will go - under 'Uncategorized Experiments'. + with this name does not exist. If no project name specified, the experiment will go + under Uncategorized Experiments. log_code (bool): Whether to log your code in your experiment (default: ``False``). log_graph (bool): Whether to log your computational graph in your experiment (default: ``False``). @@ -33,9 +34,9 @@ class CometMLLogger(LoggerDestination): to :attr:`.State.run_name`. rank_zero_only (bool, optional): Whether to log only on the rank-zero process. (default: ``False``). - init_kwargs (Dict[str, Any], optional): Any additional init kwargs - ``wandb.init`` (see - `WandB do`cumentation `_). + exp_kwargs (Dict[str, Any], optional): Any additional kwargs to + comet_ml.Experiment(see + `Comet documentation `_). """ def __init__( @@ -46,10 +47,10 @@ def __init__( log_graph: bool = False, name: Optional[str] = None, rank_zero_only: bool = True, - init_kwargs: Optional[Dict[str, Any]] = None, + exp_kwargs: Optional[Dict[str, Any]] = None, ) -> None: try: - import comet_ml + from comet_ml import Experiment except ImportError as e: raise MissingConditionalImportError(extra_deps_group='comet_ml', conda_package='comet_ml', @@ -57,23 +58,22 @@ def __init__( self._enabled = (not rank_zero_only) or dist.get_global_rank() == 0 - if init_kwargs is None: - init_kwargs = {} + if exp_kwargs is None: + exp_kwargs = {} if workspace is not None: - init_kwargs['workspace'] = workspace + exp_kwargs['workspace'] = workspace if project_name is not None: - init_kwargs['project_name'] = project_name - - init_kwargs['log_code'] = log_code - init_kwargs['log_graph'] = log_graph + exp_kwargs['project_name'] = project_name + exp_kwargs['log_code'] = log_code + exp_kwargs['log_graph'] = log_graph self.name = name self._rank_zero_only = rank_zero_only - self._init_kwargs = init_kwargs - self.experiment = Optional[comet_ml.Experiment] = None + self._exp_kwargs = exp_kwargs + self.experiment: Optional[Experiment] = None def init(self, state: State, logger: Logger) -> None: import comet_ml @@ -88,14 +88,15 @@ def init(self, state: State, logger: Logger) -> None: self.name += f'-rank{dist.get_global_rank()}' if self._enabled: - self.experiment = comet_ml.Experiment(**self._init_kwargs) + self.experiment = comet_ml.Experiment(**self._exp_kwargs) self.experiment.set_name(self.name) def log_metrics(self, metrics: Dict[str, Any], step: Optional[int] = None) -> None: if self._enabled: + assert self.experiment is not None self.experiment.log_metrics(dic=metrics, step=step) def log_hyperparameters(self, hyperparameters: Dict[str, Any]): if self._enabled: + assert self.experiment is not None self.experiment.log_parameters(hyperparameters) - diff --git a/composer/loggers/logger_hparams_registry.py b/composer/loggers/logger_hparams_registry.py index d10744243c..c0652ef6fa 100644 --- a/composer/loggers/logger_hparams_registry.py +++ b/composer/loggers/logger_hparams_registry.py @@ -14,6 +14,7 @@ import yahp as hp +from composer.loggers.cometml_logger import CometMLLogger from composer.loggers.file_logger import FileLogger from composer.loggers.in_memory_logger import InMemoryLogger from composer.loggers.logger_destination import LoggerDestination @@ -86,4 +87,5 @@ def initialize_object(self) -> ObjectStoreLogger: 'tensorboard': TensorboardLogger, 'in_memory': InMemoryLogger, 'object_store': ObjectStoreLoggerHparams, + 'cometml': CometMLLogger, } diff --git a/docs/source/getting_started/installation.rst b/docs/source/getting_started/installation.rst index e91dca419f..661f783f97 100644 --- a/docs/source/getting_started/installation.rst +++ b/docs/source/getting_started/installation.rst @@ -23,6 +23,7 @@ the following installation targets are available: * ``pip install 'mosaicml[unet]'``: Installs Composer with support for :doc:`Unet `. * ``pip install 'mosaicml[timm]'``: Installs Composer with support for :mod:`timm`. * ``pip install 'mosaicml[wandb]'``: Installs Composer with support for :mod:`wandb`. +* ``pip install 'mosaicml[comet_ml]'``: Installs Composer with support for :mod:`comet_ml`. * ``pip install 'mosaicml[all]'``: Install all optional dependencies. For a developer install, clone directly: diff --git a/docs/source/notes/run_name.md b/docs/source/notes/run_name.md index 868aa4a787..321d420511 100644 --- a/docs/source/notes/run_name.md +++ b/docs/source/notes/run_name.md @@ -53,13 +53,11 @@ See {class}`~.CheckpointSaver` for more information on specifying the arguments In addition to checkpointing, loggers also use the `run_name` for default logging. -#### Tensorboard Logger +#### Experiment Tracking Loggers -The {class}`~.TensorboardLogger` will save all the logs for a run to a folder called `run_name` and the name of each run in the Tensorboard GUI will be `run_name`. - -#### Weights and Biases Logger - -The `run_name` you specify will be used by the {class}`~.WandBLogger` as the run name for Weights and Biases. +* The {class}`~.TensorboardLogger` will save all the logs for a run to a folder called `run_name` and the name of each run in the Tensorboard GUI will be `run_name`. +* The `run_name` you specify will be used by the {class}`~.WandBLogger` as the run name for Weights and Biases. +* The `run_name` you specify will be used by the {class}`~.CometMLLogger` as the run name for your Comet experiment. #### Object Store Logger diff --git a/docs/source/trainer/logging.rst b/docs/source/trainer/logging.rst index 545d202504..9308bcfd13 100644 --- a/docs/source/trainer/logging.rst +++ b/docs/source/trainer/logging.rst @@ -45,6 +45,7 @@ Available Loggers ~file_logger.FileLogger ~wandb_logger.WandBLogger + ~cometml_logger.CometMLLogger ~progress_bar_logger.ProgressBarLogger ~tensorboard_logger.TensorboardLogger ~in_memory_logger.InMemoryLogger diff --git a/examples/run_composer_trainer.py b/examples/run_composer_trainer.py index 342d32ba83..8e8c50debc 100755 --- a/examples/run_composer_trainer.py +++ b/examples/run_composer_trainer.py @@ -42,14 +42,8 @@ def _main(): trainer = hparams.initialize_object() - # if using wandb, store the config inside the wandb run - try: - import wandb - except ImportError: - pass - else: - if wandb.run is not None: - wandb.config.update(hparams.to_dict()) + # Log all hyperparameters. + trainer.logger.log_hyperparameters(hparams.to_dict()) # Only log the config once, since it should be the same on all ranks. if dist.get_global_rank() == 0: diff --git a/setup.py b/setup.py index b6a69cd1bf..c2ea5dc4cc 100644 --- a/setup.py +++ b/setup.py @@ -144,9 +144,7 @@ def package_files(prefix: str, directory: str, extension: str): 'wandb>=0.13.2,<0.14', ] -extra_deps['comet_ml'] = [ - 'comet_ml>=3.31.12' -] +extra_deps['comet_ml'] = ['comet_ml>=3.31.12'] extra_deps['tensorboard'] = [ 'tensorboard>=2.9.1,<3.0.0', diff --git a/tests/callbacks/callback_settings.py b/tests/callbacks/callback_settings.py index faaf7d13a1..288087052d 100644 --- a/tests/callbacks/callback_settings.py +++ b/tests/callbacks/callback_settings.py @@ -14,7 +14,7 @@ from composer.callbacks.callback_hparams_registry import callback_registry from composer.callbacks.export_for_inference import ExportForInferenceCallback from composer.callbacks.mlperf import MLPerfCallback -from composer.loggers import ObjectStoreLogger, TensorboardLogger, WandBLogger +from composer.loggers import CometMLLogger, ObjectStoreLogger, TensorboardLogger, WandBLogger from composer.loggers.logger_destination import LoggerDestination from composer.loggers.logger_hparams_registry import ObjectStoreLoggerHparams, logger_registry from composer.loggers.progress_bar_logger import ProgressBarLogger @@ -35,6 +35,13 @@ except ImportError: _TENSORBOARD_INSTALLED = False +try: + import comet_ml + _COMETML_INSTALLED = True + del comet_ml # unused +except ImportError: + _COMETML_INSTALLED = False + try: import mlperf_logging _MLPERF_INSTALLED = True @@ -115,6 +122,7 @@ pytest.mark.filterwarnings( r'ignore:Specifying the ProgressBarLogger via `loggers` is deprecated:DeprecationWarning') ], + CometMLLogger: [pytest.mark.skipif(not _COMETML_INSTALLED, reason='comet_ml is optional'),], TensorboardLogger: [pytest.mark.skipif(not _TENSORBOARD_INSTALLED, reason='Tensorboard is optional'),], ObjectStoreLoggerHparams: [ pytest.mark.filterwarnings( From 39e1cc7958f10edc6918ff08298af7840b659382 Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Wed, 31 Aug 2022 20:09:20 -0700 Subject: [PATCH 04/12] change key name to comet_ml --- composer/loggers/logger_hparams_registry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer/loggers/logger_hparams_registry.py b/composer/loggers/logger_hparams_registry.py index c0652ef6fa..6a7537335c 100644 --- a/composer/loggers/logger_hparams_registry.py +++ b/composer/loggers/logger_hparams_registry.py @@ -87,5 +87,5 @@ def initialize_object(self) -> ObjectStoreLogger: 'tensorboard': TensorboardLogger, 'in_memory': InMemoryLogger, 'object_store': ObjectStoreLoggerHparams, - 'cometml': CometMLLogger, + 'comet_ml': CometMLLogger, } From 95f2883d1a79330a9678afef2d0352febe6a11f7 Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Wed, 31 Aug 2022 20:26:28 -0700 Subject: [PATCH 05/12] Add cometmlCredentialsId --- .ci/Jenkinsfile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.ci/Jenkinsfile b/.ci/Jenkinsfile index 104b0cffcb..867e105ed7 100644 --- a/.ci/Jenkinsfile +++ b/.ci/Jenkinsfile @@ -55,6 +55,7 @@ numDaysOfBuildsToKeep = '7' // number of days to keep builds (so Jenkins doesn't jenkinsShellJobName = 'scratch/command2' // The jenkins job name used to spawn sub-jobs gitCredentialsId = '9cf9add1-2cdd-414b-8160-94bd4ac4a13d' // Jenkins credential ID to use for git clones wandbCredentialsId = 'e6fbcd33-e739-483f-a4a9-9b6815572c8b' +cometmlCredentialsId = '42164c6f-e0b0-4b0e-8464-e8b71231b994' awsCredentialsId = 'd931e1a8-f356-42f4-bc4d-bc8582c3bf9e' sshCredentialsId = 'f71fda19-c867-4e7b-b6e8-3894e8288076' nodeSelector = 'mosaicml.com/instance-size=mosaic.a100-40sxm.1' @@ -271,6 +272,10 @@ void runPytest(String pDockerImage, String markers, String extraDeps, Boolean is name: 'P_JENKINS_USERNAME_PASSWORD_CREDENTIALS', value: "IGNORED_WANDB_USERNAME,WANDB_API_KEY=$wandbCredentialsId", ), + string( + name: 'P_JENKINS_USERNAME_PASSWORD_CREDENTIALS', + value: "WANDB_API_KEY=$cometmlCredentialsId", + ), string( name: 'P_JENKINS_AWS_CREDENTIALS', value: awsCredentialsId, From 5070189372a97651833bf35922ee367b9b748be7 Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Thu, 1 Sep 2022 10:26:40 -0700 Subject: [PATCH 06/12] Update setup.py Co-authored-by: Mihir Patel --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c2ea5dc4cc..945d029803 100644 --- a/setup.py +++ b/setup.py @@ -144,7 +144,7 @@ def package_files(prefix: str, directory: str, extension: str): 'wandb>=0.13.2,<0.14', ] -extra_deps['comet_ml'] = ['comet_ml>=3.31.12'] +extra_deps['comet_ml'] = ['comet_ml>=3.31.12,<4.0.0'] extra_deps['tensorboard'] = [ 'tensorboard>=2.9.1,<3.0.0', From 1de98818daea3475b58cf40c514facd99648725b Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Thu, 1 Sep 2022 14:51:41 -0700 Subject: [PATCH 07/12] init experiment in constructor --- composer/loggers/cometml_logger.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/composer/loggers/cometml_logger.py b/composer/loggers/cometml_logger.py index 59ce0bb1fe..4776eeea86 100644 --- a/composer/loggers/cometml_logger.py +++ b/composer/loggers/cometml_logger.py @@ -73,10 +73,9 @@ def __init__( self.name = name self._rank_zero_only = rank_zero_only self._exp_kwargs = exp_kwargs - self.experiment: Optional[Experiment] = None + self.experiment = Experiment(**self._exp_kwargs) def init(self, state: State, logger: Logger) -> None: - import comet_ml del logger # unused # Use the logger run name if the name is not set. @@ -88,15 +87,12 @@ def init(self, state: State, logger: Logger) -> None: self.name += f'-rank{dist.get_global_rank()}' if self._enabled: - self.experiment = comet_ml.Experiment(**self._exp_kwargs) self.experiment.set_name(self.name) def log_metrics(self, metrics: Dict[str, Any], step: Optional[int] = None) -> None: if self._enabled: - assert self.experiment is not None self.experiment.log_metrics(dic=metrics, step=step) def log_hyperparameters(self, hyperparameters: Dict[str, Any]): if self._enabled: - assert self.experiment is not None self.experiment.log_parameters(hyperparameters) From c308ab6ba861a1604c3c52848b57df1020423402 Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Wed, 7 Sep 2022 13:33:40 -0700 Subject: [PATCH 08/12] fix jenkins file --- .ci/Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/Jenkinsfile b/.ci/Jenkinsfile index 867e105ed7..47532fc76d 100644 --- a/.ci/Jenkinsfile +++ b/.ci/Jenkinsfile @@ -274,7 +274,7 @@ void runPytest(String pDockerImage, String markers, String extraDeps, Boolean is ), string( name: 'P_JENKINS_USERNAME_PASSWORD_CREDENTIALS', - value: "WANDB_API_KEY=$cometmlCredentialsId", + value: "IGNORED_COMET_USERNAME,COMET_API_KEY=$cometmlCredentialsId", ), string( name: 'P_JENKINS_AWS_CREDENTIALS', From a1bb123ac50075c92410d035dcea45030b053ccf Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Wed, 7 Sep 2022 17:00:41 -0700 Subject: [PATCH 09/12] Add unit test / integration test. --- tests/loggers/test_cometml_logger.py | 63 ++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 tests/loggers/test_cometml_logger.py diff --git a/tests/loggers/test_cometml_logger.py b/tests/loggers/test_cometml_logger.py new file mode 100644 index 0000000000..d7732f5988 --- /dev/null +++ b/tests/loggers/test_cometml_logger.py @@ -0,0 +1,63 @@ +import pytest + +import comet_ml +from pathlib import Path +import zipfile +from json import JSONDecoder +import os +from composer.utils import dist +from unittest.mock import MagicMock + + +def test_comet_ml_logging(monkeypatch, tmp_path): + """Check metrics logged with CometMLLogger are properly written to offline dump.""" + + # Set some dummy log values. + steps = [0, 1, 2] + metric_values = [0.1, 0.4, 0.7] + metric_name = 'my_test_metric' + param_names = ['my_cool_parameter1', 'my_cool_parameter2'] + param_values = [10, 3] + + # Set offline directory. + offline_directory = str(tmp_path / Path('.my_cometml_runs')) + os.environ['COMET_OFFLINE_DIRECTORY'] = offline_directory + + # Monkeypatch Experiment with OfflineExperiment to avoid uploading to CometML and + # avoid needing an API+KEY. + monkeypatch.setattr(comet_ml, "Experiment", comet_ml.OfflineExperiment) + from composer.loggers import CometMLLogger + + + # Log dummy values with CometMLLogger. + comet_logger = CometMLLogger() + comet_logger.log_hyperparameters(dict(zip(param_names, param_values))) + for step, metric_value in zip(steps, metric_values): + comet_logger.log_metrics({'my_test_metric':metric_value}, step=step) + comet_logger.experiment.end() + + + + # Open, decompress, decode, and extract offline dump of metrics. + comet_exp_dump_filepath = str(Path(offline_directory) / Path(comet_logger.experiment.id).with_suffix('.zip')) + zf = zipfile.ZipFile(comet_exp_dump_filepath) + comet_logs_path = zf.extract('messages.json', path=offline_directory) + jd = JSONDecoder() + metric_msgs = [] + param_msgs = [] + with open(comet_logs_path) as f: + for line in f.readlines(): + comet_msg = jd.decode(line) + if (comet_msg['type'] == "metric_msg") and (comet_msg['payload']['metric']['metricName'] == 'my_test_metric'): + metric_msgs.append(comet_msg['payload']['metric']) + if comet_msg['type'] == 'parameter_msg' and (comet_msg['payload']['param']["paramName"].startswith('my_cool')): + param_msgs.append(comet_msg['payload']['param']) + + + # Assert dummy metrics input to log_metrics are the same as + # those written to offline dump. + assert [msg['metricValue'] for msg in metric_msgs] == metric_values + assert [msg['step'] for msg in metric_msgs] == steps + assert all([msg['metricName'] == metric_name for msg in metric_msgs]) + assert [msg['paramValue'] for msg in param_msgs] == param_values + assert [msg['paramName'] for msg in param_msgs] == param_names \ No newline at end of file From 1a209ea028b11ce70a92d8c114ab2645e7e053d2 Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Thu, 8 Sep 2022 13:35:55 -0700 Subject: [PATCH 10/12] fix tests --- tests/callbacks/callback_settings.py | 8 +++++-- tests/loggers/test_cometml_logger.py | 34 ++++++++++++++-------------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/tests/callbacks/callback_settings.py b/tests/callbacks/callback_settings.py index 288087052d..7e34b0733c 100644 --- a/tests/callbacks/callback_settings.py +++ b/tests/callbacks/callback_settings.py @@ -20,7 +20,7 @@ from composer.loggers.progress_bar_logger import ProgressBarLogger from composer.utils.object_store.libcloud_object_store import LibcloudObjectStore from tests.common import get_module_subclasses - +import os try: import wandb _WANDB_INSTALLED = True @@ -38,10 +38,14 @@ try: import comet_ml _COMETML_INSTALLED = True + os.environ['COMET_API_KEY'] del comet_ml # unused except ImportError: _COMETML_INSTALLED = False - +# If COMET_API_KEY not set. +except KeyError: + _COMETML_INSTALLED = False + try: import mlperf_logging _MLPERF_INSTALLED = True diff --git a/tests/loggers/test_cometml_logger.py b/tests/loggers/test_cometml_logger.py index d7732f5988..9a94f9ed35 100644 --- a/tests/loggers/test_cometml_logger.py +++ b/tests/loggers/test_cometml_logger.py @@ -1,16 +1,18 @@ -import pytest +# Copyright 2022 MosaicML Composer authors +# SPDX-License-Identifier: Apache-2.0 -import comet_ml -from pathlib import Path +import os import zipfile from json import JSONDecoder -import os -from composer.utils import dist -from unittest.mock import MagicMock +from pathlib import Path + +import pytest def test_comet_ml_logging(monkeypatch, tmp_path): """Check metrics logged with CometMLLogger are properly written to offline dump.""" + pytest.importorskip('comet_ml', reason='comet_ml is optional') + import comet_ml # Set some dummy log values. steps = [0, 1, 2] @@ -22,22 +24,19 @@ def test_comet_ml_logging(monkeypatch, tmp_path): # Set offline directory. offline_directory = str(tmp_path / Path('.my_cometml_runs')) os.environ['COMET_OFFLINE_DIRECTORY'] = offline_directory - + # Monkeypatch Experiment with OfflineExperiment to avoid uploading to CometML and # avoid needing an API+KEY. - monkeypatch.setattr(comet_ml, "Experiment", comet_ml.OfflineExperiment) + monkeypatch.setattr(comet_ml, 'Experiment', comet_ml.OfflineExperiment) from composer.loggers import CometMLLogger - # Log dummy values with CometMLLogger. comet_logger = CometMLLogger() comet_logger.log_hyperparameters(dict(zip(param_names, param_values))) for step, metric_value in zip(steps, metric_values): - comet_logger.log_metrics({'my_test_metric':metric_value}, step=step) + comet_logger.log_metrics({'my_test_metric': metric_value}, step=step) comet_logger.experiment.end() - - # Open, decompress, decode, and extract offline dump of metrics. comet_exp_dump_filepath = str(Path(offline_directory) / Path(comet_logger.experiment.id).with_suffix('.zip')) zf = zipfile.ZipFile(comet_exp_dump_filepath) @@ -48,16 +47,17 @@ def test_comet_ml_logging(monkeypatch, tmp_path): with open(comet_logs_path) as f: for line in f.readlines(): comet_msg = jd.decode(line) - if (comet_msg['type'] == "metric_msg") and (comet_msg['payload']['metric']['metricName'] == 'my_test_metric'): + if (comet_msg['type'] == 'metric_msg') and (comet_msg['payload']['metric']['metricName'] + == 'my_test_metric'): metric_msgs.append(comet_msg['payload']['metric']) - if comet_msg['type'] == 'parameter_msg' and (comet_msg['payload']['param']["paramName"].startswith('my_cool')): + if comet_msg['type'] == 'parameter_msg' and ( + comet_msg['payload']['param']['paramName'].startswith('my_cool')): param_msgs.append(comet_msg['payload']['param']) - # Assert dummy metrics input to log_metrics are the same as - # those written to offline dump. + # those written to offline dump. assert [msg['metricValue'] for msg in metric_msgs] == metric_values assert [msg['step'] for msg in metric_msgs] == steps assert all([msg['metricName'] == metric_name for msg in metric_msgs]) assert [msg['paramValue'] for msg in param_msgs] == param_values - assert [msg['paramName'] for msg in param_msgs] == param_names \ No newline at end of file + assert [msg['paramName'] for msg in param_msgs] == param_names From 03ce055e265d694fae4dec843a7b0caa5c64adcd Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Thu, 8 Sep 2022 13:46:04 -0700 Subject: [PATCH 11/12] fix jenkins --- .ci/Jenkinsfile | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.ci/Jenkinsfile b/.ci/Jenkinsfile index 47532fc76d..104b0cffcb 100644 --- a/.ci/Jenkinsfile +++ b/.ci/Jenkinsfile @@ -55,7 +55,6 @@ numDaysOfBuildsToKeep = '7' // number of days to keep builds (so Jenkins doesn't jenkinsShellJobName = 'scratch/command2' // The jenkins job name used to spawn sub-jobs gitCredentialsId = '9cf9add1-2cdd-414b-8160-94bd4ac4a13d' // Jenkins credential ID to use for git clones wandbCredentialsId = 'e6fbcd33-e739-483f-a4a9-9b6815572c8b' -cometmlCredentialsId = '42164c6f-e0b0-4b0e-8464-e8b71231b994' awsCredentialsId = 'd931e1a8-f356-42f4-bc4d-bc8582c3bf9e' sshCredentialsId = 'f71fda19-c867-4e7b-b6e8-3894e8288076' nodeSelector = 'mosaicml.com/instance-size=mosaic.a100-40sxm.1' @@ -272,10 +271,6 @@ void runPytest(String pDockerImage, String markers, String extraDeps, Boolean is name: 'P_JENKINS_USERNAME_PASSWORD_CREDENTIALS', value: "IGNORED_WANDB_USERNAME,WANDB_API_KEY=$wandbCredentialsId", ), - string( - name: 'P_JENKINS_USERNAME_PASSWORD_CREDENTIALS', - value: "IGNORED_COMET_USERNAME,COMET_API_KEY=$cometmlCredentialsId", - ), string( name: 'P_JENKINS_AWS_CREDENTIALS', value: awsCredentialsId, From 95c904494281f99ace01851a772066e8c337757a Mon Sep 17 00:00:00 2001 From: Evan Racah Date: Thu, 8 Sep 2022 13:56:24 -0700 Subject: [PATCH 12/12] precommit fix --- tests/callbacks/callback_settings.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/callbacks/callback_settings.py b/tests/callbacks/callback_settings.py index 7e34b0733c..8878953fce 100644 --- a/tests/callbacks/callback_settings.py +++ b/tests/callbacks/callback_settings.py @@ -1,6 +1,7 @@ # Copyright 2022 MosaicML Composer authors # SPDX-License-Identifier: Apache-2.0 +import os from typing import Any, Dict, List, Type, Union import pytest @@ -20,7 +21,7 @@ from composer.loggers.progress_bar_logger import ProgressBarLogger from composer.utils.object_store.libcloud_object_store import LibcloudObjectStore from tests.common import get_module_subclasses -import os + try: import wandb _WANDB_INSTALLED = True @@ -45,7 +46,7 @@ # If COMET_API_KEY not set. except KeyError: _COMETML_INSTALLED = False - + try: import mlperf_logging _MLPERF_INSTALLED = True