Fix support for symlink save_dir in TensorBoardLogger (#6730)

* Add test for symlink support and initial fix

* Respond to comment and add docstring

* Update CHANGELOG.md

* Simplify

* Update pytorch_lightning/utilities/cloud_io.py

Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com>

* Make `LightningLocalFileSystem` protected

Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com>
This commit is contained in:
Ethan Harris 2021-04-06 10:36:25 +01:00 committed by GitHub
parent cf8e828559
commit 89b5326ca5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 1 deletions

View File

@ -245,6 +245,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Fixed resolve a bug with omegaconf and xm.save ([#6741](https://github.com/PyTorchLightning/pytorch-lightning/pull/6741))
- Fixed a bug where `TensorBoardLogger` would give a warning and not log correctly to a symbolic link `save_dir` ([#6730](https://github.com/PyTorchLightning/pytorch-lightning/pull/6730))
## [1.2.4] - 2021-03-16
### Changed

View File

@ -12,15 +12,29 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import os
import io
from distutils.version import LooseVersion
from pathlib import Path
from typing import IO, Union
import fsspec
from fsspec.implementations.local import LocalFileSystem
import torch
class _LightningLocalFileSystem(LocalFileSystem):
"""Extension of ``fsspec.implementations.local.LocalFileSystem`` where ``LightningLocalFileSystem.isdir`` behaves
the same as ``os.isdir``.
To be removed when https://github.com/intake/filesystem_spec/issues/591 is fixed.
"""
def isdir(self, path: str) -> bool:
return os.path.isdir(path) # follows symlinks
def load(path_or_url: Union[str, IO, Path], map_location=None):
if not isinstance(path_or_url, (str, Path)):
# any sort of BytesIO or similiar
@ -39,7 +53,7 @@ def get_filesystem(path: Union[str, Path]):
return fsspec.filesystem(path.split(":", 1)[0])
else:
# use local filesystem
return fsspec.filesystem("file")
return _LightningLocalFileSystem()
def atomic_save(checkpoint, filepath: str):

View File

@ -301,3 +301,22 @@ def test_tensorboard_save_hparams_to_yaml_once(tmpdir):
hparams_file = "hparams.yaml"
assert os.path.isfile(os.path.join(trainer.log_dir, hparams_file))
assert not os.path.isfile(os.path.join(tmpdir, hparams_file))
@mock.patch('pytorch_lightning.loggers.tensorboard.log')
def test_tensorboard_with_symlink(log, tmpdir):
"""
Tests a specific failure case when tensorboard logger is used with empty name, symbolic link ``save_dir``, and
relative paths.
"""
os.chdir(tmpdir) # need to use relative paths
source = os.path.join('.', 'lightning_logs')
dest = os.path.join('.', 'sym_lightning_logs')
os.makedirs(source, exist_ok=True)
os.symlink(source, dest)
logger = TensorBoardLogger(save_dir=dest, name='')
_ = logger.version
log.warning.assert_not_called()