Add np.ndarray as a recognized type for TB histograms. #1635

iwishiwasaneagle · 2023-07-28T17:26:27Z

Currently the SB3 tensorboard writer only supports torch.Tensor as a histogram value. However, the SummaryWriter actually also allows np.ndarray as a value. This PR enables this.

Motivation and Context

Closes #1634

I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (update in the documentation)

Checklist

Note: You can run most of the checks using make commit-checks.

Note: we are using a maximum length of 127 characters per line

Torch histograms allow th.Tensor, np.ndarray, and caffe2 formatted strings. This commits expands the TensorBoardOutputFormat's capabilities to log the two former types.

iwishiwasaneagle · 2023-08-09T12:19:58Z

This has been fixed in pytorch v2.0.0 and I'll look into how to get this working for earlier versions for this repo, if at all possible, to ensure compatibility with pytorch>=1.13.0 as it currently is. According to the numpy docs, this is a deprecation issue.

From my testing, this works perfectly fine with numpy 1.23.0 and torch 1.13.1

Proof that it's numpy

numpy 1.23.0

# Dockerfile
FROM python:3.11

COPY ./setup.py /src/setup.py
COPY ./stable_baselines3/version.txt /src/stable_baselines3/version.txt

WORKDIR /src

RUN pip install torch==1.13+cpu -f https://download.pytorch.org/whl/torch_stable.html \ 
	numpy==1.23.0 \
	tensorboard \
	.[tests]

CMD /bin/bash

$ docker build .  -t sb3-dev -f Dockerfile
$ docker run -v $PWD:/src/stable-baselines3 --rm sb3-dev python -m pytest /src/stable-baselines3/tests/test_logger.py
============================= test session starts ==============================
platform linux -- Python 3.11.4, pytest-7.4.0, pluggy-1.2.0
rootdir: /src/stable-baselines3
configfile: pyproject.toml
plugins: cov-4.1.0, xdist-3.3.1, env-0.8.2
collected 50 items

stable-baselines3/tests/test_logger.py ................................. [ 66%]
.................                                                        [100%]

=============================== warnings summary ===============================
../usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/__init__.py:4
  /usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/__init__.py:4: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    if not hasattr(tensorboard, "__version__") or LooseVersion(

../usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/__init__.py:6
  /usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/__init__.py:6: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    ) < LooseVersion("1.15"):

tests/test_logger.py::test_make_output[tensorboard]
tests/test_logger.py::test_make_output[tensorboard]
tests/test_logger.py::test_make_output[tensorboard]
tests/test_logger.py::test_report_histogram_to_tensorboard[histogram0]
tests/test_logger.py::test_report_histogram_to_tensorboard[histogram1]
  /usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/summary.py:386: DeprecationWarning: using `dtype=` in comparisons is only useful for `dtype=object` (and will do nothing for bool). This operation will fail in the future.
    cum_counts = np.cumsum(np.greater(counts, 0, dtype=np.int32))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================== 50 passed, 7 warnings in 2.43s ========================

numpy 1.24.0

# Dockerfile
FROM python:3.11

COPY ./setup.py /src/setup.py
COPY ./stable_baselines3/version.txt /src/stable_baselines3/version.txt

WORKDIR /src

RUN pip install torch==1.13+cpu -f https://download.pytorch.org/whl/torch_stable.html \ 
	numpy==1.23.0 \
	tensorboard \
	.[tests]

CMD /bin/bash

$ docker build .  -t sb3-dev -f Dockerfile
$ docker run -v $PWD:/src/stable-baselines3 --rm sb3-dev python -m pytest /src/stable-baselines3/tests/test_logger.py
============================= test session starts ==============================
platform linux -- Python 3.11.4, pytest-7.4.0, pluggy-1.2.0
rootdir: /src/stable-baselines3
configfile: pyproject.toml
plugins: cov-4.1.0, xdist-3.3.1, env-0.8.2
collected 50 items

stable-baselines3/tests/test_logger.py ......F...........FF............. [ 66%]
.................                                                        [100%]

=================================== FAILURES ===================================

Relevant changes in pytorch

v1.13.1
https://github.com/pytorch/pytorch/blame/49444c3e546bf240bed24a101e747422d1f8a0ee/torch/utils/tensorboard/summary.py#L386

v2.0.0
https://github.com/pytorch/pytorch/blame/c263bd43e8e8502d4726643bc6fd046f0130ac0e/torch/utils/tensorboard/summary.py#L383

iwishiwasaneagle · 2023-08-31T08:21:50Z

@araffin I could just wrap the code in a try-catch until SB3 supports torch >= 2.0.0?

Something like

try:
	self.writer.add_histogram(key, value, step)
except TypeError:
	pass

which would still work in the original manner, whilst letting people with newer versions of torch leverage this feature. Then open a tracker issue to ensure it's not forgotten about.

araffin · 2023-08-31T08:27:44Z

Something like

Probably cast to torch tensor automatically (using from_numpy()) and output a warning too?

…ons. See DLR-RM#1635 for more details

iwishiwasaneagle · 2023-08-31T09:10:56Z

@araffin Good idea. Okay that should be that fixed with your suggestions implemented. I tested the new proposed solution in the same manner as outlined here and I saw the warning (the deprecation from numpy and from the try/except) but the tests passed. Running test_logger.py with coverage enabled showed that all branches are being hit which should bullet proof this solution against regression. Once SB3 supports torch>=2.0.0 the relevant code can be reverted back to d37a952.

Log:

$ docker run -v $PWD:/src/stable-baselines3 --rm sb3-dev python -m pytest /src/stable-baselines3/tests/test_logger.py
============================= test session starts ==============================
platform linux -- Python 3.11.5, pytest-7.4.0, pluggy-1.3.0
rootdir: /src/stable-baselines3
configfile: pyproject.toml
plugins: cov-4.1.0, env-1.0.1, xdist-3.3.1
collected 51 items

stable-baselines3/tests/test_logger.py ................................. [ 64%]
..................                                                       [100%]

=============================== warnings summary ===============================
../usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/__init__.py:4
  /usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/__init__.py:4: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    if not hasattr(tensorboard, "__version__") or LooseVersion(

../usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/__init__.py:6
  /usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/__init__.py:6: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    ) < LooseVersion("1.15"):

tests/test_logger.py::test_make_output[tensorboard]
tests/test_logger.py::test_make_output[tensorboard]
tests/test_logger.py::test_make_output[tensorboard]
tests/test_logger.py::test_report_histogram_to_tensorboard[histogram0-False]
tests/test_logger.py::test_report_histogram_to_tensorboard[histogram1-False]
tests/test_logger.py::test_report_histogram_to_tensorboard[histogram2-True]
  /usr/local/lib/python3.11/site-packages/torch/utils/tensorboard/summary.py:386: DeprecationWarning: using `dtype=` in comparisons is only useful for `dtype=object` (and will do nothing for bool). This operation will fail in the future.
    cum_counts = np.cumsum(np.greater(counts, 0, dtype=np.int32))

tests/test_logger.py::test_report_histogram_to_tensorboard[histogram2-True]
  /src/stable-baselines3/stable_baselines3/common/logger.py:419: UserWarning: A numpy.ndarray was passed to write which threw a TypeError. This is most likely due to an outdated numpy version (<1.24.0) and/or an outdated torch version (<2.0.0). The ndarray will be converted to a torch.Tensor as a workaround. For more information, see https://github.com/DLR-RM/stable-baselines3/pull/1635
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================== 51 passed, 9 warnings in 3.58s ========================

araffin · 2023-08-31T09:38:19Z

tests/test_logger.py

+_called = None
+
+
+def get_fail_first_then_pass_fn(fn, exception=Exception):


you should not need that, just record the warnings with pytest and check that the correct warning is there (we have some examples in the tests)

Well I guess with the current CI setup this warning is always hit. However, this test will then fail for anyone with newer versions of np and/pr torch.

you should be able to check the version of pytorch to know if a warning should be outputted or not?

So would you propose a check before the add_histogram to see if a warning and conversion is needed?

araffin · 2023-08-31T09:40:46Z

tests/test_logger.py

+    pytest.importorskip("tensorboard")
+
+    writer = make_output_format("tensorboard", tmp_path)
+    writer.write({"data": histogram}, key_excluded={"data": ()})


I'm not sure if the key excluded is needed here

It's a required parameter for all KVWriter subclasses AFAIK

stable-baselines3/stable_baselines3/common/logger.py

Line 117 in 84163b4

def write(self, key_values: Dict[str, Any], key_excluded: Dict[str, Tuple[str, ...]], step: int = 0) -> None:

araffin · 2023-08-31T09:42:30Z

tests/test_logger.py

+    writer = make_output_format("tensorboard", tmp_path)
+    writer.write({"data": histogram}, key_excluded={"data": ()})
+
+    assert all("Histogram" not in f for f in read_log("tensorboard").lines)


maybe add a comment, something like "check that the values were not logged as histogram"
(I'm not sure if all of them are logged btw)

See 383ee76

…t have been called

iwishiwasaneagle and others added 4 commits July 28, 2023 17:53

Add np.ndarray as a recognized type for TB histograms.

fec692c

Torch histograms allow th.Tensor, np.ndarray, and caffe2 formatted strings. This commits expands the TensorBoardOutputFormat's capabilities to log the two former types.

Update changelog to reflect bug fix

d37a952

Merge branch 'master' into master

f0620e3

Merge branch 'master' into master

185d63b

araffin added the Maintainers on vacation Maintainers are on vacation so they can recharge their batteries, we will be back soon ;) label Aug 9, 2023

iwishiwasaneagle added 2 commits August 21, 2023 16:29

Merge branch 'master' into master

253bb64

Merge branch 'master' into master

523fe36

araffin self-requested a review August 30, 2023 09:13

araffin removed the Maintainers on vacation Maintainers are on vacation so they can recharge their batteries, we will be back soon ;) label Aug 30, 2023

Merge branch 'master' into master

d101bcd

fix: try/catch for if either np or torch aren't at the required versi…

4386be5

…ons. See DLR-RM#1635 for more details

araffin reviewed Aug 31, 2023

View reviewed changes

iwishiwasaneagle and others added 5 commits August 31, 2023 10:50

fix: Add comment describing the test for when add_histogram should no…

383ee76

…t have been called

Merge branch 'master' into master

0343155

Merge branch 'master' into master

fca8112

Merge branch 'master' into master

7fb3135

Merge branch 'master' into master

5121148

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add np.ndarray as a recognized type for TB histograms. #1635

Add np.ndarray as a recognized type for TB histograms. #1635

iwishiwasaneagle commented Jul 28, 2023 •

edited

iwishiwasaneagle commented Aug 9, 2023

iwishiwasaneagle commented Aug 31, 2023

araffin commented Aug 31, 2023

iwishiwasaneagle commented Aug 31, 2023

araffin Aug 31, 2023

iwishiwasaneagle Aug 31, 2023

araffin Aug 31, 2023

iwishiwasaneagle Aug 31, 2023

araffin Aug 31, 2023 •

edited

iwishiwasaneagle Aug 31, 2023

araffin Aug 31, 2023

iwishiwasaneagle Aug 31, 2023

		_called = None


		def get_fail_first_then_pass_fn(fn, exception=Exception):

Add np.ndarray as a recognized type for TB histograms. #1635

Are you sure you want to change the base?

Add np.ndarray as a recognized type for TB histograms. #1635

Conversation

iwishiwasaneagle commented Jul 28, 2023 • edited

Motivation and Context

Types of changes

Checklist

iwishiwasaneagle commented Aug 9, 2023

iwishiwasaneagle commented Aug 31, 2023

araffin commented Aug 31, 2023

iwishiwasaneagle commented Aug 31, 2023

araffin Aug 31, 2023

Choose a reason for hiding this comment

iwishiwasaneagle Aug 31, 2023

Choose a reason for hiding this comment

araffin Aug 31, 2023

Choose a reason for hiding this comment

iwishiwasaneagle Aug 31, 2023

Choose a reason for hiding this comment

araffin Aug 31, 2023 • edited

Choose a reason for hiding this comment

iwishiwasaneagle Aug 31, 2023

Choose a reason for hiding this comment

araffin Aug 31, 2023

Choose a reason for hiding this comment

iwishiwasaneagle Aug 31, 2023

Choose a reason for hiding this comment

iwishiwasaneagle commented Jul 28, 2023 •

edited

araffin Aug 31, 2023 •

edited