Skip to content

Commit

Permalink
Lazy initialize tensorboard
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #4852

Every time a summary writer object is created it'll end up creating a file in the passed in logdir with just metadata of 40 bytes. This will cause a lot of spam with every rank initializing the file aggressively (I've done this myself when adding support for Adhoc logging to PyPer to a lot of angry complaints).

Report at https://fb.workplace.com/groups/723537759122220/posts/745046770304652/

Sample code that accesses "_writer" directly: https://www.internalfb.com/code/fbsource/[6b39948cd8e179fcc49f08526efa5dc26fc00843]/fbcode/mobile-vision/d2go/d2go/utils/visualization.py?lines=180

Reviewed By: Reubend, crassirostris

Differential Revision: D43863071

fbshipit-source-id: 3fb749199772837697911d802d7e43d157597573
  • Loading branch information
kunalb authored and facebook-github-bot committed Apr 7, 2023
1 parent d779ea6 commit 1bc3a33
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 3 deletions.
11 changes: 8 additions & 3 deletions detectron2/utils/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import time
from collections import defaultdict
from contextlib import contextmanager
from functools import cached_property
from typing import Optional
import torch
from fvcore.common.history_buffer import HistoryBuffer
Expand Down Expand Up @@ -142,10 +143,14 @@ def __init__(self, log_dir: str, window_size: int = 20, **kwargs):
kwargs: other arguments passed to `torch.utils.tensorboard.SummaryWriter(...)`
"""
self._window_size = window_size
self._writer_args = {"log_dir": log_dir, **kwargs}
self._last_write = -1

@cached_property
def _writer(self):
from torch.utils.tensorboard import SummaryWriter

self._writer = SummaryWriter(log_dir, **kwargs)
self._last_write = -1
return SummaryWriter(**self._writer_args)

def write(self):
storage = get_event_storage()
Expand Down Expand Up @@ -174,7 +179,7 @@ def write(self):
storage.clear_histograms()

def close(self):
if hasattr(self, "_writer"): # doesn't exist when the code fails at import
if "_writer" in self.__dict__:
self._writer.close()


Expand Down
23 changes: 23 additions & 0 deletions tests/utils/test_tensorboardx.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import os
import tempfile
import unittest

from detectron2.utils.events import TensorboardXWriter


# TODO Fix up capitalization
class TestTensorboardXWriter(unittest.TestCase):
def test_no_files_created(self) -> None:
with tempfile.TemporaryDirectory() as tmp_dir:
writer = TensorboardXWriter(tmp_dir)
writer.close()

self.assertFalse(os.listdir(tmp_dir))

def test_single_write(self) -> None:
with tempfile.TemporaryDirectory() as tmp_dir:
writer = TensorboardXWriter(tmp_dir)
writer._writer.add_scalar("testing", 1, 1)
writer.close()

self.assertTrue(os.listdir(tmp_dir))

1 comment on commit 1bc3a33

@fedllanes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

functools.cached_property is compatible only with python 3.8 and newer, however this project should be compatible with python 3.7 and newer.
Shouldn't this be changed? @kunalb

Please sign in to comment.