Repository: pytorch/ignite

Issue:
support for torch.utils.tensorboard when using `tensorboard_logger`
With PyTorch 1.1.0, TensorBoard is now [natively supported](https://pytorch.org/docs/stable/tensorboard.html) in PyTorch. 

Can we add a new option to switch `torch.utils.tensorboard` or `tensorboardX`?

I found that `tensorboard_logger` only need `SummaryWriter` to implement `add_histogram` and `add_scalar`. It seems that we can just import `torch.utils.tensorboard` when import tensorboardX failed.

```python
try:
    from tensorboardX import SummaryWriter
except ImportError:
    from torch.utils.tensorboard import SummaryWriter
```

For now, tensorboardX:
> Support scalar, image, figure, histogram, audio, text, graph, onnx_graph, embedding, pr_curve, mesh and video summaries.

`torch.utils.tensorboard`:
> Scalars, images, histograms, graphs, and embedding visualizations are all supported for PyTorch models and tensors as well as Caffe2 nets and blobs.



Discussion:
bosr: Note: I took the liberty to remove the `test_no_tensorboardX` test as it did not seem relevant anymore. As an alternative, we could check that the `SummaryWriter` class is indeed from PyTorch?
vfdev-5: @bosr thanks for the PR ! 

> As an alternative, we could check that the SummaryWriter class is indeed from PyTorch?

Yes, I think we will need this, otherwise coverage wont be "happy" with the code coverage delta...

vfdev-5: @bosr do you need any help with tests ?
bosr: Ah ah, why not ;) I’ll write to you this evening. You saw my multiple failures on my fake branch? 

Romain

> On 10 Sep 2019, at 11:50, vfdev <notifications@github.com> wrote:
> 
> ﻿
> @bosr do you need any help with tests ?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

vfdev-5: @bosr no problems.
bosr: Hi, I've just rebased and included some support for the test.

Basically, my problem is that, if I re-establish the test, like for instance:

    with TensorboardLogger(log_dir=dirname) as tb_logger:
        assert(isinstance(tb_logger.writer, torch.utils.tensorboard.SummaryWriter), \
            "in the absence of tensorboardX, SummaryWriter should come from torch.utils.tensorboard"

then there is another `ImportError` about missing `tensorboard` module in `torch.utils`. I'm honestly surprised by this error. You can see it here: https://travis-ci.org/bosr/ignite/jobs/583263107

Do you have an idea for fixing this? Or do you prefer to cancel this and just write a warning?

Thanks for your support! 
vfdev-5: > there is another ImportError about missing tensorboard module in torch.utils. I'm honestly surprised by this error. 

This is because `tensorboard` is not a declared module : https://github.com/pytorch/pytorch/blob/master/torch/utils/__init__.py

Maybe you can do something like : 
```python

from torch.utils.tensorboard import SummaryWriter
with TensorboardLogger(log_dir=dirname) as tb_logger:
        assert isinstance(tb_logger.writer, SummaryWriter), type(tb_logger.writer)
```
bosr: Oops, I got confused with the import, thanks. I’ll update the PR. 
bosr: I've finally installed the test suite locally, so I'll keep iterating and come back here. Thanks!
bosr: Thanks a lot for the fixes! The code is nicer now, and one more case covered. This incentivized me to push it a little bit further and reduce to:

```python
def test_no_tensorboardX_package(dirname):
    from torch.utils.tensorboard import SummaryWriter
    with patch.dict('sys.modules', {'tensorboardX': None}):
        tb_logger = TensorboardLogger(log_dir=dirname)
        assert isinstance(tb_logger.writer, SummaryWriter), type(tb_logger.writer)

def test_no_torch_utils_tensorboard_package(dirname):
    from tensorboardX import SummaryWriter
    with patch.dict('sys.modules', {'torch.utils.tensorboard': None}):
        tb_logger = TensorboardLogger(log_dir=dirname)
        assert isinstance(tb_logger.writer, SummaryWriter), type(tb_logger.writer)


def test_no_tensorboardX_nor_torch_utils_tensorboard():
    with patch.dict('sys.modules', {'tensorboardX': None, 'torch.utils.tensorboard': None}):
        with pytest.raises(RuntimeError, match=r'This contrib module requires either tensorboardX or torch'):
            TensorboardLogger(log_dir=None)
```
replacing all related fixtures. Locally it passes on both 2.7 and 3.6+. Let's see if CI agrees ;)

I kept the previous code as comments for discussion. Tell me what you think about it.

Patch:
diff --git a/.travis.yml b/.travis.yml
index 5b4ac4e5..69c7d420 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -29,7 +29,7 @@ before_install: &before_install
   - source activate test-environment
   - if [[ $TRAVIS_PYTHON_VERSION == 2.7 ]]; then pip install enum34; fi
   # Test contrib dependencies
-  - pip install tqdm scikit-learn tensorboardX visdom polyaxon-client mlflow
+  - pip install tqdm scikit-learn tensorboardX visdom polyaxon-client mlflow tensorboard
   - if [[ $TRAVIS_PYTHON_VERSION == 3.6 ]]; then pip install pynvml; fi
   # Futures should be already installed via visdom -> tornado -> futures
   # Let's reinstall it anyway to be sure
diff --git a/ignite/contrib/handlers/tensorboard_logger.py b/ignite/contrib/handlers/tensorboard_logger.py
index b1cb76f0..1a0c0759 100644
--- a/ignite/contrib/handlers/tensorboard_logger.py
+++ b/ignite/contrib/handlers/tensorboard_logger.py
@@ -322,12 +322,13 @@ class TensorboardLogger(BaseLogger):
     """
     TensorBoard handler to log metrics, model/optimizer parameters, gradients during the training and validation.
 
-    This class requires `tensorboardX <https://github.com/lanpa/tensorboardX>`_ package to be installed:
+    By default, this class favors `tensorboardX <https://github.com/lanpa/tensorboardX>`_ package if installed:
 
     .. code-block:: bash
 
         pip install tensorboardX
 
+    otherwise, it falls back to using PyTorch's SummaryWriter (>=v1.2.0).
 
     Args:
         *args: Positional arguments accepted from :class:`~tensorboardx.SummaryWriter`.
@@ -413,8 +414,12 @@ class TensorboardLogger(BaseLogger):
         try:
             from tensorboardX import SummaryWriter
         except ImportError:
-            raise RuntimeError("This contrib module requires tensorboardX to be installed. "
-                               "Please install it with command: \n pip install tensorboardX")
+            try:
+                from torch.utils.tensorboard import SummaryWriter
+            except ImportError:
+                raise RuntimeError("This contrib module requires either tensorboardX or torch >= 1.2.0. "
+                                   "You may install tensorboardX with command: \n pip install tensorboardX \n"
+                                   "or upgrade PyTorch using your package manager of choice (pip or conda).")
 
         self.writer = SummaryWriter(*args, **kwargs)
 


Test patch:
diff --git a/tests/ignite/contrib/handlers/test_tensorboard_logger.py b/tests/ignite/contrib/handlers/test_tensorboard_logger.py
index 70e2ba77..0ec40c5c 100644
--- a/tests/ignite/contrib/handlers/test_tensorboard_logger.py
+++ b/tests/ignite/contrib/handlers/test_tensorboard_logger.py
@@ -3,7 +3,7 @@ import math
 
 import pytest
 
-from mock import MagicMock, call, ANY, Mock
+from mock import MagicMock, call, ANY, Mock, patch
 
 import torch
 
@@ -560,44 +560,88 @@ def test_integration_as_context_manager(dirname):
     assert len(written_files) > 0
 
 
-@pytest.fixture
-def no_site_packages():
-    import sys
-    tensorboardX_module = sys.modules['tensorboardX']
-    del sys.modules['tensorboardX']
-    prev_path = list(sys.path)
-    sys.path = [p for p in sys.path if "site-packages" not in p]
-    yield "no_site_packages"
-    sys.path = prev_path
-    sys.modules['tensorboardX'] = tensorboardX_module
+# @pytest.fixture
+# def no_tensorboardX_package():
+#     import sys
+#     import tensorboardX
+#     from torch.utils import tensorboard
+#     # remove tensorboardX
+#     tensorboardX_module = sys.modules['tensorboardX']
+#     del sys.modules['tensorboardX']
+#     # save and replace sys.path
+#     prev_path = list(sys.path)
+#     sys.path = [p for p in sys.path if "site-packages" not in p]
+#     yield "no_site_packages"
+#     # put everything back in place
+#     sys.path = prev_path
+#     sys.modules['tensorboardX'] = tensorboardX_module
 
 
-def test_no_tensorboardX(dirname, no_site_packages):
+# def test_no_tensorboardX(dirname, no_tensorboardX_package):
 
-    with pytest.raises(RuntimeError, match=r"This contrib module requires tensorboardX to be installed"):
-        TensorboardLogger(log_dir=dirname)
+#     from torch.utils.tensorboard import SummaryWriter
+#     with TensorboardLogger(log_dir=dirname) as tb_logger:
+#         assert isinstance(tb_logger.writer, SummaryWriter), type(tb_logger.writer)
 
 
-@pytest.fixture
-def mock_tb_module():
-    import sys
-    import types
+# @pytest.fixture
+# def mock_tb_module():
+#     import sys
+#     import types
 
-    module_name = 'tensorboardX'
-    tb_module = types.ModuleType(module_name)
-    prev_tb_module = sys.modules[module_name]
-    sys.modules[module_name] = tb_module
+#     module_name = 'tensorboardX'
+#     tb_module = types.ModuleType(module_name)
+#     prev_tb_module = sys.modules[module_name]
+#     sys.modules[module_name] = tb_module
 
-    yield tb_module
-    sys.modules[module_name] = prev_tb_module
+#     yield tb_module
+#     sys.modules[module_name] = prev_tb_module
 
 
-def test_init_typeerror_exception(mock_tb_module):
+# def test_init_typeerror_exception(mock_tb_module):
 
-    def side_effect(*args, **kwargs):
-        raise TypeError("a problem")
+#     def side_effect(*args, **kwargs):
+#         raise TypeError("a problem")
 
-    mock_tb_module.SummaryWriter = Mock(name='tensorboardX.SummaryWriter', side_effect=side_effect)
+#     mock_tb_module.SummaryWriter = Mock(name='tensorboardX.SummaryWriter', side_effect=side_effect)
 
-    with pytest.raises(TypeError, match=r'a problem'):
-        TensorboardLogger(log_dir=None)
+#     with pytest.raises(TypeError, match=r'a problem'):
+#         TensorboardLogger(log_dir=None)
+
+
+# @pytest.fixture
+# def mock_no_torch_utils_tensorboard():
+#     import sys
+#     from torch.utils import tensorboard
+#     orig_module = sys.modules['torch.utils.tensorboard']
+#     mock_module = Mock(spec=tensorboard)
+#     del mock_module.SummaryWriter
+#     sys.modules['torch.utils.tensorboard'] = mock_module
+#     yield mock_module
+#     sys.modules['torch.utils.tensorboard'] = orig_module
+
+
+# def test_no_torch_utils_tensorboard(no_tensorboardX_package, mock_no_torch_utils_tensorboard):
+
+#     with pytest.raises(RuntimeError, match=r'This contrib module requires either tensorboardX or torch'):
+#         TensorboardLogger(log_dir=None)
+
+
+def test_no_tensorboardX_package(dirname):
+    from torch.utils.tensorboard import SummaryWriter
+    with patch.dict('sys.modules', {'tensorboardX': None}):
+        tb_logger = TensorboardLogger(log_dir=dirname)
+        assert isinstance(tb_logger.writer, SummaryWriter), type(tb_logger.writer)
+
+
+def test_no_torch_utils_tensorboard_package(dirname):
+    from tensorboardX import SummaryWriter
+    with patch.dict('sys.modules', {'torch.utils.tensorboard': None}):
+        tb_logger = TensorboardLogger(log_dir=dirname)
+        assert isinstance(tb_logger.writer, SummaryWriter), type(tb_logger.writer)
+
+
+def test_no_tensorboardX_nor_torch_utils_tensorboard():
+    with patch.dict('sys.modules', {'tensorboardX': None, 'torch.utils.tensorboard': None}):
+        with pytest.raises(RuntimeError, match=r'This contrib module requires either tensorboardX or torch'):
+            TensorboardLogger(log_dir=None)