From 100092cd8f785f074e89e37d1d8de26937d9e93c Mon Sep 17 00:00:00 2001 From: Thomas Chaton Date: Wed, 16 Jun 2021 15:59:48 -0400 Subject: [PATCH 1/9] better name traces --- pytorch_lightning/profiler/base.py | 19 +++++++++++++------ pytorch_lightning/profiler/pytorch.py | 5 +++-- tests/test_profiler.py | 4 ++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/pytorch_lightning/profiler/base.py b/pytorch_lightning/profiler/base.py index 2a064085e8da7..2e7db1411c078 100644 --- a/pytorch_lightning/profiler/base.py +++ b/pytorch_lightning/profiler/base.py @@ -113,14 +113,20 @@ def _rank_zero_info(self, *args, **kwargs) -> None: if self._local_rank in (None, 0): log.info(*args, **kwargs) + def _add_suffix(self, filename: str, suffix: Optional[str], token: str = '-') -> str: + if suffix is None: + return filename + + if len(filename) >= 1 and filename[-1] != token: + filename += token + return filename + suffix + def _prepare_filename(self, extension: str = ".txt") -> str: filename = "" - if self._stage is not None: - filename += f"{self._stage}-" - filename += str(self.filename) - if self._local_rank is not None: - filename += f"-{self._local_rank}" - filename += extension + filename = self._add_suffix(filename, self._stage.value if self._stage else None) + filename = self._add_suffix(filename, self.filename if self.filename else None) + filename = self._add_suffix(filename, str(self._local_rank) if self._local_rank is not None else None) + filename += extension if extension else '' return filename def _prepare_streams(self) -> None: @@ -168,6 +174,7 @@ def setup( """Execute arbitrary pre-profiling set-up steps.""" self._stage = stage self._local_rank = local_rank + print("LOCAL_RANK 1", self._local_rank) self._log_dir = log_dir self.dirpath = self.dirpath or log_dir diff --git a/pytorch_lightning/profiler/pytorch.py b/pytorch_lightning/profiler/pytorch.py index b78922d7f4a47..737809ce4c188 100644 --- a/pytorch_lightning/profiler/pytorch.py +++ b/pytorch_lightning/profiler/pytorch.py @@ -426,12 +426,13 @@ def stop(self, action_name: str) -> None: def on_trace_ready(profiler): if self.dirpath is not None: + filename = self._prepare_filename(extension="") if self._export_to_chrome: - handler = tensorboard_trace_handler(self.dirpath, self._prepare_filename(extension="")) + handler = tensorboard_trace_handler(self.dirpath, filename + "-" + str(action_name)) handler(profiler) if self._export_to_flame_graph: - path = os.path.join(self.dirpath, self._prepare_filename(extension=".stack")) + path = os.path.join(self.dirpath, filename + "-" + str(action_name) + ".stack") profiler.export_stacks(path, metric=self._metric) else: rank_zero_warn("The PyTorchProfiler failed to export trace as `dirpath` is None") diff --git a/tests/test_profiler.py b/tests/test_profiler.py index acc2bac1c466f..d940d4426b4a6 100644 --- a/tests/test_profiler.py +++ b/tests/test_profiler.py @@ -331,8 +331,8 @@ def test_pytorch_profiler_trainer_ddp(tmpdir, pytorch_profiler): files = [file for file in files if file.endswith('.json')] assert len(files) == 2, files local_rank = trainer.local_rank - assert any(f'training_step_{local_rank}' in f for f in files) - assert any(f'validation_step_{local_rank}' in f for f in files) + assert any(f'{local_rank}-training_step_and_backward' in f for f in files) + assert any(f'{local_rank}-validation_step' in f for f in files) def test_pytorch_profiler_trainer_test(tmpdir): From ae1698e620cb20dbdc1a793abc8f6af2c7c57010 Mon Sep 17 00:00:00 2001 From: Thomas Chaton Date: Wed, 16 Jun 2021 16:01:48 -0400 Subject: [PATCH 2/9] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fb3acd19bf01..fa9759f9e4698 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -264,6 +264,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed `BaseFinetuning` callback to properly handle parent modules w/ parameters ([#7931](https://github.com/PyTorchLightning/pytorch-lightning/pull/7931)) +- Fixed `PyTorchProfiler` generated filenames ([#8009](https://github.com/PyTorchLightning/pytorch-lightning/pull/8009)) + + ## [1.3.5] - 2021-06-08 ### Added From 4548cdc795bd7bf9d7a26036df3417367f0710a7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 16 Jun 2021 20:02:05 +0000 Subject: [PATCH 3/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pytorch_lightning/profiler/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytorch_lightning/profiler/base.py b/pytorch_lightning/profiler/base.py index 2e7db1411c078..5d014fcb0fedc 100644 --- a/pytorch_lightning/profiler/base.py +++ b/pytorch_lightning/profiler/base.py @@ -116,7 +116,7 @@ def _rank_zero_info(self, *args, **kwargs) -> None: def _add_suffix(self, filename: str, suffix: Optional[str], token: str = '-') -> str: if suffix is None: return filename - + if len(filename) >= 1 and filename[-1] != token: filename += token return filename + suffix From 9564aab3924c88be365881664b47ed1306a67822 Mon Sep 17 00:00:00 2001 From: Thomas Chaton Date: Wed, 16 Jun 2021 16:02:53 -0400 Subject: [PATCH 4/9] remove print --- pytorch_lightning/profiler/base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pytorch_lightning/profiler/base.py b/pytorch_lightning/profiler/base.py index 5d014fcb0fedc..348781a0c154b 100644 --- a/pytorch_lightning/profiler/base.py +++ b/pytorch_lightning/profiler/base.py @@ -174,7 +174,6 @@ def setup( """Execute arbitrary pre-profiling set-up steps.""" self._stage = stage self._local_rank = local_rank - print("LOCAL_RANK 1", self._local_rank) self._log_dir = log_dir self.dirpath = self.dirpath or log_dir From 7b84b12c26c310067e89ceb08c4329ab429b6963 Mon Sep 17 00:00:00 2001 From: Thomas Chaton Date: Fri, 18 Jun 2021 13:03:22 -0400 Subject: [PATCH 5/9] update --- CHANGELOG.md | 6 +++--- pytorch_lightning/profiler/base.py | 27 ++++++++++++++------------- pytorch_lightning/profiler/pytorch.py | 5 ++--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa9759f9e4698..ed9011cc3515f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -185,6 +185,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - `LightningCLI` now aborts with a clearer message if config already exists and disables save config during `fast_dev_run`([#7963](https://github.com/PyTorchLightning/pytorch-lightning/pull/7963)) +- Improve `PyTorchProfiler` chrome traces names ([#8009](https://github.com/PyTorchLightning/pytorch-lightning/pull/8009)) + + ### Deprecated @@ -264,9 +267,6 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed `BaseFinetuning` callback to properly handle parent modules w/ parameters ([#7931](https://github.com/PyTorchLightning/pytorch-lightning/pull/7931)) -- Fixed `PyTorchProfiler` generated filenames ([#8009](https://github.com/PyTorchLightning/pytorch-lightning/pull/8009)) - - ## [1.3.5] - 2021-06-08 ### Added diff --git a/pytorch_lightning/profiler/base.py b/pytorch_lightning/profiler/base.py index 348781a0c154b..42836e641052b 100644 --- a/pytorch_lightning/profiler/base.py +++ b/pytorch_lightning/profiler/base.py @@ -113,20 +113,21 @@ def _rank_zero_info(self, *args, **kwargs) -> None: if self._local_rank in (None, 0): log.info(*args, **kwargs) - def _add_suffix(self, filename: str, suffix: Optional[str], token: str = '-') -> str: - if suffix is None: - return filename - - if len(filename) >= 1 and filename[-1] != token: - filename += token - return filename + suffix - - def _prepare_filename(self, extension: str = ".txt") -> str: + def _prepare_filename(self, action_name: Optional[str] = None, extension: str = ".txt", separation_token: str = "-") -> str: filename = "" - filename = self._add_suffix(filename, self._stage.value if self._stage else None) - filename = self._add_suffix(filename, self.filename if self.filename else None) - filename = self._add_suffix(filename, str(self._local_rank) if self._local_rank is not None else None) - filename += extension if extension else '' + token = "" + if self._stage: + filename += f'{token}{self._stage.value}' + token = separation_token + if self.filename: + filename += f'{token}{self.filename}' + token = separation_token + if self._local_rank is not None: + filename += f'{token}{self._local_rank}' + token = separation_token + if action_name: + filename += f'{token}{action_name}' + filename += extension return filename def _prepare_streams(self) -> None: diff --git a/pytorch_lightning/profiler/pytorch.py b/pytorch_lightning/profiler/pytorch.py index 737809ce4c188..59fda8d556427 100644 --- a/pytorch_lightning/profiler/pytorch.py +++ b/pytorch_lightning/profiler/pytorch.py @@ -426,13 +426,12 @@ def stop(self, action_name: str) -> None: def on_trace_ready(profiler): if self.dirpath is not None: - filename = self._prepare_filename(extension="") if self._export_to_chrome: - handler = tensorboard_trace_handler(self.dirpath, filename + "-" + str(action_name)) + handler = tensorboard_trace_handler(self.dirpath, self._prepare_filename(action_name=action_name, extension="")) handler(profiler) if self._export_to_flame_graph: - path = os.path.join(self.dirpath, filename + "-" + str(action_name) + ".stack") + path = os.path.join(self.dirpath, self._prepare_filename(action_name=action_name, extension=".stack")) profiler.export_stacks(path, metric=self._metric) else: rank_zero_warn("The PyTorchProfiler failed to export trace as `dirpath` is None") From 13bfbe56cc2f74df260f833adc8e4c4229fd3db4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 18 Jun 2021 17:13:40 +0000 Subject: [PATCH 6/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pytorch_lightning/profiler/base.py | 4 +++- pytorch_lightning/profiler/pytorch.py | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pytorch_lightning/profiler/base.py b/pytorch_lightning/profiler/base.py index a9abb3d079f36..c2d63d513bdd6 100644 --- a/pytorch_lightning/profiler/base.py +++ b/pytorch_lightning/profiler/base.py @@ -112,7 +112,9 @@ def _rank_zero_info(self, *args, **kwargs) -> None: if self._local_rank in (None, 0): log.info(*args, **kwargs) - def _prepare_filename(self, action_name: Optional[str] = None, extension: str = ".txt", separation_token: str = "-") -> str: + def _prepare_filename( + self, action_name: Optional[str] = None, extension: str = ".txt", separation_token: str = "-" + ) -> str: filename = "" token = "" if self._stage: diff --git a/pytorch_lightning/profiler/pytorch.py b/pytorch_lightning/profiler/pytorch.py index e53c3e7634a84..0c0bde515a4dd 100644 --- a/pytorch_lightning/profiler/pytorch.py +++ b/pytorch_lightning/profiler/pytorch.py @@ -427,11 +427,15 @@ def stop(self, action_name: str) -> None: def on_trace_ready(profiler): if self.dirpath is not None: if self._export_to_chrome: - handler = tensorboard_trace_handler(self.dirpath, self._prepare_filename(action_name=action_name, extension="")) + handler = tensorboard_trace_handler( + self.dirpath, self._prepare_filename(action_name=action_name, extension="") + ) handler(profiler) if self._export_to_flame_graph: - path = os.path.join(self.dirpath, self._prepare_filename(action_name=action_name, extension=".stack")) + path = os.path.join( + self.dirpath, self._prepare_filename(action_name=action_name, extension=".stack") + ) profiler.export_stacks(path, metric=self._metric) else: rank_zero_warn("The PyTorchProfiler failed to export trace as `dirpath` is None") From 7da144adfd1fe2ad63db7624e0c7eb23c5167dc9 Mon Sep 17 00:00:00 2001 From: Carlos Mocholi Date: Fri, 18 Jun 2021 19:29:50 +0200 Subject: [PATCH 7/9] Refactor --- pytorch_lightning/profiler/base.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pytorch_lightning/profiler/base.py b/pytorch_lightning/profiler/base.py index c2d63d513bdd6..317f8001197b6 100644 --- a/pytorch_lightning/profiler/base.py +++ b/pytorch_lightning/profiler/base.py @@ -113,22 +113,18 @@ def _rank_zero_info(self, *args, **kwargs) -> None: log.info(*args, **kwargs) def _prepare_filename( - self, action_name: Optional[str] = None, extension: str = ".txt", separation_token: str = "-" + self, action_name: Optional[str] = None, extension: str = ".txt", split_token: str = "-" ) -> str: - filename = "" - token = "" + args = [] if self._stage: - filename += f'{token}{self._stage.value}' - token = separation_token + args.append(self._stage) if self.filename: - filename += f'{token}{self.filename}' - token = separation_token + args.append(self.filename) if self._local_rank is not None: - filename += f'{token}{self._local_rank}' - token = separation_token - if action_name: - filename += f'{token}{action_name}' - filename += extension + args.append(self._local_rank) + if action_name is not None: + args.append(action_name) + filename = split_token.join(args) + extension return filename def _prepare_streams(self) -> None: From f620b51bcce14f915327db52670eda66f296440f Mon Sep 17 00:00:00 2001 From: Carlos Mocholi Date: Fri, 18 Jun 2021 19:30:34 +0200 Subject: [PATCH 8/9] is not None --- pytorch_lightning/profiler/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytorch_lightning/profiler/base.py b/pytorch_lightning/profiler/base.py index 317f8001197b6..fb7929d8e7437 100644 --- a/pytorch_lightning/profiler/base.py +++ b/pytorch_lightning/profiler/base.py @@ -116,7 +116,7 @@ def _prepare_filename( self, action_name: Optional[str] = None, extension: str = ".txt", split_token: str = "-" ) -> str: args = [] - if self._stage: + if self._stage is not None: args.append(self._stage) if self.filename: args.append(self.filename) From 831bd17166cc64b5c8002e5705a32467734564dc Mon Sep 17 00:00:00 2001 From: tchaton Date: Fri, 18 Jun 2021 18:52:41 +0100 Subject: [PATCH 9/9] resolve test --- pytorch_lightning/profiler/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytorch_lightning/profiler/base.py b/pytorch_lightning/profiler/base.py index fb7929d8e7437..8b5bf5483d976 100644 --- a/pytorch_lightning/profiler/base.py +++ b/pytorch_lightning/profiler/base.py @@ -121,7 +121,7 @@ def _prepare_filename( if self.filename: args.append(self.filename) if self._local_rank is not None: - args.append(self._local_rank) + args.append(str(self._local_rank)) if action_name is not None: args.append(action_name) filename = split_token.join(args) + extension