Skip to content

Commit

Permalink
Remove confusing warning "Missing logger folder" (#20109)
Browse files Browse the repository at this point in the history
  • Loading branch information
awaelchli authored Jul 20, 2024
1 parent 1cd7741 commit e214395
Show file tree
Hide file tree
Showing 6 changed files with 2 additions and 45 deletions.
1 change: 0 additions & 1 deletion src/lightning/fabric/loggers/csv_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ def _get_next_version(self) -> int:
versions_root = os.path.join(self._root_dir, self.name)

if not _is_dir(self._fs, versions_root, strict=True):
log.warning("Missing logger folder: %s", versions_root)
return 0

existing_versions = []
Expand Down
5 changes: 0 additions & 5 deletions src/lightning/fabric/loggers/tensorboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import logging
import os
from argparse import Namespace
from typing import TYPE_CHECKING, Any, Dict, Mapping, Optional, Union
Expand All @@ -30,8 +29,6 @@
from lightning.fabric.utilities.types import _PATH
from lightning.fabric.wrappers import _unwrap_objects

log = logging.getLogger(__name__)

_TENSORBOARD_AVAILABLE = RequirementCache("tensorboard")
_TENSORBOARDX_AVAILABLE = RequirementCache("tensorboardX")
if TYPE_CHECKING:
Expand Down Expand Up @@ -305,8 +302,6 @@ def _get_next_version(self) -> int:
try:
listdir_info = self._fs.listdir(save_dir)
except OSError:
# TODO(fabric): This message can be confusing (did user do something wrong?). Improve it or remove it.
log.warning("Missing logger folder: %s", save_dir)
return 0

existing_versions = []
Expand Down
3 changes: 0 additions & 3 deletions src/lightning/pytorch/loggers/csv_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"""

import logging
import os
from argparse import Namespace
from typing import Any, Dict, Optional, Union
Expand All @@ -35,8 +34,6 @@
from lightning.pytorch.loggers.logger import Logger
from lightning.pytorch.utilities.rank_zero import rank_zero_only

log = logging.getLogger(__name__)


class ExperimentWriter(_FabricExperimentWriter):
r"""Experiment writer for CSVLogger.
Expand Down
4 changes: 0 additions & 4 deletions src/lightning/pytorch/loggers/tensorboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
------------------
"""

import logging
import os
from argparse import Namespace
from typing import Any, Dict, Optional, Union
Expand All @@ -36,8 +35,6 @@
from lightning.pytorch.utilities.imports import _OMEGACONF_AVAILABLE
from lightning.pytorch.utilities.rank_zero import rank_zero_only, rank_zero_warn

log = logging.getLogger(__name__)


class TensorBoardLogger(Logger, FabricTensorBoardLogger):
r"""Log to local or remote file system in `TensorBoard <https://www.tensorflow.org/tensorboard>`_ format.
Expand Down Expand Up @@ -245,7 +242,6 @@ def _get_next_version(self) -> int:
try:
listdir_info = self._fs.listdir(root_dir)
except OSError:
log.warning("Missing logger folder: %s", root_dir)
return 0

existing_versions = []
Expand Down
17 changes: 1 addition & 16 deletions tests/tests_fabric/loggers/test_tensorboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
import os
from argparse import Namespace
from unittest import mock
Expand Down Expand Up @@ -213,8 +212,7 @@ def test_tensorboard_finalize(monkeypatch, tmp_path):
logger.experiment.close.assert_called()


@mock.patch("lightning.fabric.loggers.tensorboard.log")
def test_tensorboard_with_symlink(log, tmp_path, monkeypatch):
def test_tensorboard_with_symlink(tmp_path, monkeypatch):
"""Tests a specific failure case when tensorboard logger is used with empty name, symbolic link ``save_dir``, and
relative paths."""
monkeypatch.chdir(tmp_path) # need to use relative paths
Expand All @@ -226,16 +224,3 @@ def test_tensorboard_with_symlink(log, tmp_path, monkeypatch):

logger = TensorBoardLogger(root_dir=dest, name="")
_ = logger.version

log.warning.assert_not_called()


def test_tensorboard_missing_folder_warning(tmp_path, caplog):
"""Verify that the logger throws a warning for invalid directory."""
name = "fake_dir"
logger = TensorBoardLogger(root_dir=tmp_path, name=name)

with caplog.at_level(logging.WARNING):
assert logger.version == 0

assert "Missing logger folder:" in caplog.text
17 changes: 1 addition & 16 deletions tests/tests_pytorch/loggers/test_tensorboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
import os
from argparse import Namespace
from unittest import mock
Expand Down Expand Up @@ -312,8 +311,7 @@ def test_tensorboard_save_hparams_to_yaml_once(tmp_path):
assert not os.path.isfile(os.path.join(tmp_path, hparams_file))


@mock.patch("lightning.pytorch.loggers.tensorboard.log")
def test_tensorboard_with_symlink(log, tmp_path, monkeypatch):
def test_tensorboard_with_symlink(tmp_path, monkeypatch):
"""Tests a specific failure case when tensorboard logger is used with empty name, symbolic link ``save_dir``, and
relative paths."""
monkeypatch.chdir(tmp_path) # need to use relative paths
Expand All @@ -325,16 +323,3 @@ def test_tensorboard_with_symlink(log, tmp_path, monkeypatch):

logger = TensorBoardLogger(save_dir=dest, name="")
_ = logger.version

log.warning.assert_not_called()


def test_tensorboard_missing_folder_warning(tmp_path, caplog):
"""Verify that the logger throws a warning for invalid directory."""
name = "fake_dir"
logger = TensorBoardLogger(save_dir=tmp_path, name=name)

with caplog.at_level(logging.WARNING):
assert logger.version == 0

assert "Missing logger folder:" in caplog.text

0 comments on commit e214395

Please sign in to comment.