From 10608e0daf089e6492cfb5e447decaf9e3a45fab Mon Sep 17 00:00:00 2001 From: rwesterman Date: Fri, 8 Nov 2019 10:32:35 -0600 Subject: [PATCH 1/8] Fixing comet ml bug and adding functionality --- pytorch_lightning/logging/comet_logger.py | 60 +++++++++++++++++++++-- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/pytorch_lightning/logging/comet_logger.py b/pytorch_lightning/logging/comet_logger.py index 5e1d281c8d9a2..88deb305cceae 100644 --- a/pytorch_lightning/logging/comet_logger.py +++ b/pytorch_lightning/logging/comet_logger.py @@ -1,15 +1,45 @@ +from torch import is_tensor + try: from comet_ml import Experiment as CometExperiment + from comet_ml.papi import API except ImportError: raise ImportError('Missing comet_ml package.') from .base import LightningLoggerBase, rank_zero_only - class CometLogger(LightningLoggerBase): - def __init__(self, *args, **kwargs): + def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, experiment_name=None, *args, **kwargs): + """ + Initialize a Comet.ml logger + + :param api_key: API key, found on Comet.ml + :param workspace: Name of workspace for this user + :param project_name: Optional. Send your experiment to a specific project. + Otherwise will be sent to Uncategorized Experiments. + If project name does not already exists Comet.ml will create a new project. + :param rest_api_key: Optional. Rest API key found in Comet.ml settings. This is used to determine version number + :param experiment_name: Optional. String representing the name for this particular experiment on Comet.ml + """ super(CometLogger, self).__init__() - self.experiment = CometExperiment(*args, **kwargs) + self.experiment = CometExperiment(api_key=api_key, workspace=workspace, project_name=project_name, *args, **kwargs) + + self.workspace = workspace + self.project_name = project_name + + if rest_api_key is not None: + # Comet.ml rest API, used to determine version number + self.rest_api_key = rest_api_key + self.comet_api = API(self.rest_api_key) + else: + self.rest_api_key = None + self.comet_api = None + + if experiment_name: + try: + self._set_experiment_name(experiment_name) + except TypeError as e: + print("Failed to set experiment name for comet.ml logger") @rank_zero_only def log_hyperparams(self, params): @@ -17,9 +47,29 @@ def log_hyperparams(self, params): @rank_zero_only def log_metrics(self, metrics, step_num): - # self.experiment.set_epoch(self, metrics.get('epoch', 0)) - self.experiment.log_metrics(metrics) + # Comet.ml expects metrics to be a dictionary of detached tensors on CPU + for key, val in metrics.items(): + if is_tensor(val): + metrics[key] = val.cpu().detach() + + self.experiment.log_metrics(metrics, step=step_num) @rank_zero_only def finalize(self, status): self.experiment.end() + + @property + def name(self): + return self.experiment.project_name + + @property + def version(self): + if self.project_name and self.rest_api_key: + # Determines the number of experiments in this project, and returns the next integer as the version number + nb_exps = len(self.comet_api.get_experiments(self.workspace, self.project_name)) + return nb_exps + 1 + else: + return None + + def _set_experiment_name(self, experiment_name): + self.experiment.set_name(experiment_name) \ No newline at end of file From f186733eea34a63fef35607a0ba71ca6827e5d24 Mon Sep 17 00:00:00 2001 From: rwesterman Date: Fri, 8 Nov 2019 10:45:19 -0600 Subject: [PATCH 2/8] Updating documents --- docs/Trainer/Logging.md | 5 ++++- pytorch_lightning/logging/comet_logger.py | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/Trainer/Logging.md b/docs/Trainer/Logging.md index 5b44384711ea2..531fda3fadb7d 100644 --- a/docs/Trainer/Logging.md +++ b/docs/Trainer/Logging.md @@ -92,7 +92,10 @@ from pytorch_lightning.logging import CometLogger # arguments made to CometLogger are passed on to the comet_ml.Experiment class comet_logger = CometLogger( api_key=os.environ["COMET_KEY"], - workspace=os.environ["COMET_KEY"], + workspace=os.environ["COMET_WORKSPACE"], + project_name="default_project", # Optional + rest_api_key=os.environ["COMET_REST_KEY"], # Optional + experiment_name = "default" # Optional ) trainer = Trainer(logger=comet_logger) ``` diff --git a/pytorch_lightning/logging/comet_logger.py b/pytorch_lightning/logging/comet_logger.py index 88deb305cceae..44d75b39d1718 100644 --- a/pytorch_lightning/logging/comet_logger.py +++ b/pytorch_lightning/logging/comet_logger.py @@ -58,6 +58,10 @@ def log_metrics(self, metrics, step_num): def finalize(self, status): self.experiment.end() + @rank_zero_only + def _set_experiment_name(self, experiment_name): + self.experiment.set_name(experiment_name) + @property def name(self): return self.experiment.project_name @@ -71,5 +75,3 @@ def version(self): else: return None - def _set_experiment_name(self, experiment_name): - self.experiment.set_name(experiment_name) \ No newline at end of file From d0da773a32c5e2ea39a331464a685a3d0c205362 Mon Sep 17 00:00:00 2001 From: rwesterman Date: Fri, 8 Nov 2019 12:06:08 -0600 Subject: [PATCH 3/8] Fixing code style issues in comet_logger --- pytorch_lightning/logging/comet_logger.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pytorch_lightning/logging/comet_logger.py b/pytorch_lightning/logging/comet_logger.py index 44d75b39d1718..bd95408b9fa82 100644 --- a/pytorch_lightning/logging/comet_logger.py +++ b/pytorch_lightning/logging/comet_logger.py @@ -8,6 +8,7 @@ from .base import LightningLoggerBase, rank_zero_only + class CometLogger(LightningLoggerBase): def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, experiment_name=None, *args, **kwargs): """ @@ -22,7 +23,8 @@ def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, exp :param experiment_name: Optional. String representing the name for this particular experiment on Comet.ml """ super(CometLogger, self).__init__() - self.experiment = CometExperiment(api_key=api_key, workspace=workspace, project_name=project_name, *args, **kwargs) + self.experiment = CometExperiment(api_key=api_key, workspace=workspace, project_name=project_name, *args, + **kwargs) self.workspace = workspace self.project_name = project_name @@ -74,4 +76,3 @@ def version(self): return nb_exps + 1 else: return None - From fbe38802d97ab9df6ad6237f3d69d03d4098a17c Mon Sep 17 00:00:00 2001 From: rwesterman Date: Fri, 8 Nov 2019 17:55:23 -0600 Subject: [PATCH 4/8] Changing comet_logger experiment to execute lazily --- docs/Trainer/Logging.md | 2 +- pytorch_lightning/logging/comet_logger.py | 28 ++++++++++++++++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/docs/Trainer/Logging.md b/docs/Trainer/Logging.md index 531fda3fadb7d..fb4d54e7b1108 100644 --- a/docs/Trainer/Logging.md +++ b/docs/Trainer/Logging.md @@ -95,7 +95,7 @@ comet_logger = CometLogger( workspace=os.environ["COMET_WORKSPACE"], project_name="default_project", # Optional rest_api_key=os.environ["COMET_REST_KEY"], # Optional - experiment_name = "default" # Optional + experiment_name="default" # Optional ) trainer = Trainer(logger=comet_logger) ``` diff --git a/pytorch_lightning/logging/comet_logger.py b/pytorch_lightning/logging/comet_logger.py index bd95408b9fa82..e5245fbc78ea1 100644 --- a/pytorch_lightning/logging/comet_logger.py +++ b/pytorch_lightning/logging/comet_logger.py @@ -1,16 +1,16 @@ -from torch import is_tensor - try: from comet_ml import Experiment as CometExperiment from comet_ml.papi import API except ImportError: raise ImportError('Missing comet_ml package.') +from torch import is_tensor + from .base import LightningLoggerBase, rank_zero_only class CometLogger(LightningLoggerBase): - def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, experiment_name=None, *args, **kwargs): + def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, experiment_name=None, **kwargs): """ Initialize a Comet.ml logger @@ -22,13 +22,15 @@ def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, exp :param rest_api_key: Optional. Rest API key found in Comet.ml settings. This is used to determine version number :param experiment_name: Optional. String representing the name for this particular experiment on Comet.ml """ - super(CometLogger, self).__init__() - self.experiment = CometExperiment(api_key=api_key, workspace=workspace, project_name=project_name, *args, - **kwargs) + super().__init__() + self._experiment = None + self.api_key = api_key self.workspace = workspace self.project_name = project_name + self._kwargs = kwargs + if rest_api_key is not None: # Comet.ml rest API, used to determine version number self.rest_api_key = rest_api_key @@ -43,6 +45,20 @@ def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, exp except TypeError as e: print("Failed to set experiment name for comet.ml logger") + @property + def experiment(self): + if self._experiment is not None: + return self._experiment + + self._experiment = CometExperiment( + api_key=self.api_key, + workspace=self.workspace, + project_name=self.project_name, + **self._kwargs + ) + + return self._experiment + @rank_zero_only def log_hyperparams(self, params): self.experiment.log_parameters(vars(params)) From 51a9d006e3615a557ce8cf6b6646a977b4dd3a4a Mon Sep 17 00:00:00 2001 From: rwesterman Date: Sun, 10 Nov 2019 13:48:43 -0600 Subject: [PATCH 5/8] Adding tests for comet_logger and addressing comments from @Borda --- pytorch_lightning/logging/comet_logger.py | 17 +++--- tests/test_y_logging.py | 66 +++++++++++++++++++++++ 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/pytorch_lightning/logging/comet_logger.py b/pytorch_lightning/logging/comet_logger.py index e5245fbc78ea1..b0ed773ef863e 100644 --- a/pytorch_lightning/logging/comet_logger.py +++ b/pytorch_lightning/logging/comet_logger.py @@ -1,3 +1,5 @@ +from logging import getLogger + try: from comet_ml import Experiment as CometExperiment from comet_ml.papi import API @@ -8,19 +10,22 @@ from .base import LightningLoggerBase, rank_zero_only +logger = getLogger(__name__) + class CometLogger(LightningLoggerBase): def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, experiment_name=None, **kwargs): """ Initialize a Comet.ml logger - :param api_key: API key, found on Comet.ml - :param workspace: Name of workspace for this user - :param project_name: Optional. Send your experiment to a specific project. + :param str api_key: API key, found on Comet.ml + :param str workspace: Name of workspace for this user + :param str project_name: Optional. Send your experiment to a specific project. Otherwise will be sent to Uncategorized Experiments. If project name does not already exists Comet.ml will create a new project. - :param rest_api_key: Optional. Rest API key found in Comet.ml settings. This is used to determine version number - :param experiment_name: Optional. String representing the name for this particular experiment on Comet.ml + :param str rest_api_key: Optional. Rest API key found in Comet.ml settings. + This is used to determine version number + :param str experiment_name: Optional. String representing the name for this particular experiment on Comet.ml """ super().__init__() self._experiment = None @@ -43,7 +48,7 @@ def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, exp try: self._set_experiment_name(experiment_name) except TypeError as e: - print("Failed to set experiment name for comet.ml logger") + logger.exception("Failed to set experiment name for comet.ml logger") @property def experiment(self): diff --git a/tests/test_y_logging.py b/tests/test_y_logging.py index c44a9e2c8b987..a28384e8bc529 100644 --- a/tests/test_y_logging.py +++ b/tests/test_y_logging.py @@ -137,6 +137,72 @@ def test_mlflow_pickle(): testing_utils.clear_save_dir() +def test_comet_logger(): + """ + verify that basic functionality of Comet.ml logger works + """ + reset_seed() + + try: + from pytorch_lightning.logging import CometLogger + except ModuleNotFoundError: + return + + hparams = testing_utils.get_hparams() + model = LightningTestModel(hparams) + + # API key for dummy Comet.ml account + logger = CometLogger( + api_key="KnmgASRHHyxWXOpwUfgrAFz8C", + project_name="general", + workspace="dummy-test", + ) + + trainer_options = dict( + max_nb_epochs=1, + train_percent_check=0.01, + logger=logger + ) + + trainer = Trainer(**trainer_options) + result = trainer.fit(model) + + print('result finished') + assert result == 1, "Training failed" + + +def test_comet_pickle(): + """ + verify that pickling trainer with mlflow logger works + """ + reset_seed() + + try: + from pytorch_lightning.logging import CometLogger + except ModuleNotFoundError: + return + + hparams = testing_utils.get_hparams() + model = LightningTestModel(hparams) + + # API key for dummy Comet.ml account + logger = CometLogger( + api_key="KnmgASRHHyxWXOpwUfgrAFz8C", + project_name="general", + workspace="dummy-test" + ) + + trainer_options = dict( + max_nb_epochs=1, + logger=logger + ) + + trainer = Trainer(**trainer_options) + pkl_bytes = pickle.dumps(trainer) + trainer2 = pickle.loads(pkl_bytes) + trainer2.logger.log_metrics({"acc": 1.0}) + + def test_custom_logger(tmpdir): class CustomLogger(LightningLoggerBase): From c2872c3228891aa564eadd1785b42c6371df974f Mon Sep 17 00:00:00 2001 From: rwesterman Date: Sun, 10 Nov 2019 14:22:56 -0600 Subject: [PATCH 6/8] Setting step_num to optional keyword argument in log_metrics() to comply to other loggers --- pytorch_lightning/logging/comet_logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytorch_lightning/logging/comet_logger.py b/pytorch_lightning/logging/comet_logger.py index b0ed773ef863e..83861e4292bd7 100644 --- a/pytorch_lightning/logging/comet_logger.py +++ b/pytorch_lightning/logging/comet_logger.py @@ -69,7 +69,7 @@ def log_hyperparams(self, params): self.experiment.log_parameters(vars(params)) @rank_zero_only - def log_metrics(self, metrics, step_num): + def log_metrics(self, metrics, step_num=None): # Comet.ml expects metrics to be a dictionary of detached tensors on CPU for key, val in metrics.items(): if is_tensor(val): From 0c01ee45d53db041492a24b211682af2a38c1a9d Mon Sep 17 00:00:00 2001 From: rwesterman Date: Mon, 11 Nov 2019 16:15:42 -0600 Subject: [PATCH 7/8] Adding offline logging mode for comet_ml, updating tests and docs --- .run_local_tests.sh | 1 + docs/Trainer/Logging.md | 17 ++++++- pytorch_lightning/logging/comet_logger.py | 60 ++++++++++++++++------- tests/test_y_logging.py | 23 ++++++--- 4 files changed, 76 insertions(+), 25 deletions(-) diff --git a/.run_local_tests.sh b/.run_local_tests.sh index 57d40c26b0ee5..0b06d104f5f70 100644 --- a/.run_local_tests.sh +++ b/.run_local_tests.sh @@ -2,6 +2,7 @@ rm -rf _ckpt_* rm -rf tests/save_dir* rm -rf tests/mlruns_* +rm -rf tests/cometruns* rm -rf tests/tests/* rm -rf lightning_logs coverage run --source pytorch_lightning -m py.test pytorch_lightning tests pl_examples -v --doctest-modules diff --git a/docs/Trainer/Logging.md b/docs/Trainer/Logging.md index fb4d54e7b1108..7dc0fde355e5b 100644 --- a/docs/Trainer/Logging.md +++ b/docs/Trainer/Logging.md @@ -87,12 +87,27 @@ def any_lightning_module_function_or_hook(...): Log using [comet](https://www.comet.ml) +Comet logger can be used in either online or offline mode. +To log in online mode, CometLogger requries an API key: ```{.python} from pytorch_lightning.logging import CometLogger # arguments made to CometLogger are passed on to the comet_ml.Experiment class comet_logger = CometLogger( api_key=os.environ["COMET_KEY"], - workspace=os.environ["COMET_WORKSPACE"], + workspace=os.environ["COMET_WORKSPACE"], # Optional + project_name="default_project", # Optional + rest_api_key=os.environ["COMET_REST_KEY"], # Optional + experiment_name="default" # Optional +) +trainer = Trainer(logger=comet_logger) +``` +To log in offline mode, CometLogger requires a path to a local directory: +```{.python} +from pytorch_lightning.logging import CometLogger +# arguments made to CometLogger are passed on to the comet_ml.Experiment class +comet_logger = CometLogger( + save_dir=".", + workspace=os.environ["COMET_WORKSPACE"], # Optional project_name="default_project", # Optional rest_api_key=os.environ["COMET_REST_KEY"], # Optional experiment_name="default" # Optional diff --git a/pytorch_lightning/logging/comet_logger.py b/pytorch_lightning/logging/comet_logger.py index 83861e4292bd7..680762614fb9d 100644 --- a/pytorch_lightning/logging/comet_logger.py +++ b/pytorch_lightning/logging/comet_logger.py @@ -2,6 +2,7 @@ try: from comet_ml import Experiment as CometExperiment + from comet_ml import OfflineExperiment as CometOfflineExperiment from comet_ml.papi import API except ImportError: raise ImportError('Missing comet_ml package.') @@ -14,12 +15,14 @@ class CometLogger(LightningLoggerBase): - def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, experiment_name=None, **kwargs): + def __init__(self, api_key=None, save_dir=None, workspace=None, + rest_api_key=None, project_name=None, experiment_name=None, **kwargs): """ - Initialize a Comet.ml logger + Initialize a Comet.ml logger. Requires either an API Key (online mode) or a local directory path (offline mode) - :param str api_key: API key, found on Comet.ml - :param str workspace: Name of workspace for this user + :param str api_key: Required in online mode. API key, found on Comet.ml + :param str save_dir: Required in offline mode. The path for the directory to save local comet logs + :param str workspace: Optional. Name of workspace for this user :param str project_name: Optional. Send your experiment to a specific project. Otherwise will be sent to Uncategorized Experiments. If project name does not already exists Comet.ml will create a new project. @@ -30,10 +33,25 @@ def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, exp super().__init__() self._experiment = None - self.api_key = api_key + # Determine online or offline mode based on which arguments were passed to CometLogger + if save_dir is not None and api_key is not None: + # If arguments are passed for both save_dir and api_key, preference is given to online mode + self.mode = "online" + self.api_key = api_key + elif api_key is not None: + self.mode = "online" + self.api_key = api_key + elif save_dir is not None: + self.mode = "offline" + self.save_dir = save_dir + else: + # If neither api_key nor save_dir are passed as arguments, raise an exception + raise Exception("CometLogger requires either api_key or save_dir during initialization.") + + logger.info(f"CometLogger will be initialized in {self.mode} mode") + self.workspace = workspace self.project_name = project_name - self._kwargs = kwargs if rest_api_key is not None: @@ -46,7 +64,7 @@ def __init__(self, api_key, workspace, rest_api_key=None, project_name=None, exp if experiment_name: try: - self._set_experiment_name(experiment_name) + self.name = experiment_name except TypeError as e: logger.exception("Failed to set experiment name for comet.ml logger") @@ -55,12 +73,20 @@ def experiment(self): if self._experiment is not None: return self._experiment - self._experiment = CometExperiment( - api_key=self.api_key, - workspace=self.workspace, - project_name=self.project_name, - **self._kwargs - ) + if self.mode == "online": + self._experiment = CometExperiment( + api_key=self.api_key, + workspace=self.workspace, + project_name=self.project_name, + **self._kwargs + ) + else: + self._experiment = CometOfflineExperiment( + offline_directory=self.save_dir, + workspace=self.workspace, + project_name=self.project_name, + **self._kwargs + ) return self._experiment @@ -81,14 +107,14 @@ def log_metrics(self, metrics, step_num=None): def finalize(self, status): self.experiment.end() - @rank_zero_only - def _set_experiment_name(self, experiment_name): - self.experiment.set_name(experiment_name) - @property def name(self): return self.experiment.project_name + @name.setter + def name(self, value): + self.experiment.set_name(value) + @property def version(self): if self.project_name and self.rest_api_key: diff --git a/tests/test_y_logging.py b/tests/test_y_logging.py index a28384e8bc529..cfa979956c6b0 100644 --- a/tests/test_y_logging.py +++ b/tests/test_y_logging.py @@ -151,9 +151,12 @@ def test_comet_logger(): hparams = testing_utils.get_hparams() model = LightningTestModel(hparams) - # API key for dummy Comet.ml account + root_dir = os.path.dirname(os.path.realpath(__file__)) + comet_dir = os.path.join(root_dir, "cometruns") + + # We test CometLogger in offline mode with local saves logger = CometLogger( - api_key="KnmgASRHHyxWXOpwUfgrAFz8C", + save_dir=comet_dir, project_name="general", workspace="dummy-test", ) @@ -170,10 +173,12 @@ def test_comet_logger(): print('result finished') assert result == 1, "Training failed" + testing_utils.clear_save_dir() + def test_comet_pickle(): """ - verify that pickling trainer with mlflow logger works + verify that pickling trainer with comet logger works """ reset_seed() @@ -185,11 +190,14 @@ def test_comet_pickle(): hparams = testing_utils.get_hparams() model = LightningTestModel(hparams) - # API key for dummy Comet.ml account + root_dir = os.path.dirname(os.path.realpath(__file__)) + comet_dir = os.path.join(root_dir, "cometruns") + + # We test CometLogger in offline mode with local saves logger = CometLogger( - api_key="KnmgASRHHyxWXOpwUfgrAFz8C", + save_dir=comet_dir, project_name="general", - workspace="dummy-test" + workspace="dummy-test", ) trainer_options = dict( @@ -202,9 +210,10 @@ def test_comet_pickle(): trainer2 = pickle.loads(pkl_bytes) trainer2.logger.log_metrics({"acc": 1.0}) + testing_utils.clear_save_dir() -def test_custom_logger(tmpdir): +def test_custom_logger(tmpdir): class CustomLogger(LightningLoggerBase): def __init__(self): super().__init__() From 18c706d718fbfaca3f001a44d2b00d31722d9c4f Mon Sep 17 00:00:00 2001 From: rwesterman Date: Mon, 11 Nov 2019 17:25:37 -0600 Subject: [PATCH 8/8] Switching to MisconfigurationException --- pytorch_lightning/logging/comet_logger.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pytorch_lightning/logging/comet_logger.py b/pytorch_lightning/logging/comet_logger.py index 680762614fb9d..21dad421b7520 100644 --- a/pytorch_lightning/logging/comet_logger.py +++ b/pytorch_lightning/logging/comet_logger.py @@ -10,6 +10,7 @@ from torch import is_tensor from .base import LightningLoggerBase, rank_zero_only +from ..utilities.debugging import MisconfigurationException logger = getLogger(__name__) @@ -46,7 +47,7 @@ def __init__(self, api_key=None, save_dir=None, workspace=None, self.save_dir = save_dir else: # If neither api_key nor save_dir are passed as arguments, raise an exception - raise Exception("CometLogger requires either api_key or save_dir during initialization.") + raise MisconfigurationException("CometLogger requires either api_key or save_dir during initialization.") logger.info(f"CometLogger will be initialized in {self.mode} mode")