[RLlib] Turn doc tests into '.. doctest::' #37492

ArturNiederfahrenhorst · 2023-07-17T23:26:53Z

Why are these changes needed?

As part of our migration to unify code snippets and doc test style (https://docs.ray.io/en/master/ray-contribute/writing-code-snippets.html#how-to-handle-hard-to-test-examples), this PR migrates our code examples in RLlib.

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

ArturNiederfahrenhorst · 2023-07-18T00:40:09Z

rllib/algorithms/callbacks.py

-                MyCustomTraceCallbacks,
-                ....
-            ]))
+    The resulting DefaultCallbacks will call all the sub-callbacks' callbacks


Let's just keep AlgorithConfig out of the picture.
The example we would have to construct to make this a copy/paste thing would be quite large.

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

bveeramani · 2023-07-18T23:04:44Z

rllib/connectors/connector.py

-                "agent_2": np.array(...),
-            }
-        )
+        .. testcode::


We need a newline after this or the example won't render.

bveeramani · 2023-07-18T23:06:38Z

rllib/connectors/connector.py

-            }
-        )
+        .. testcode::
+            import numpy as np


Rather than having one large testcode, I'd recommend splitting this into multiple testcode for better readability:

Represent a list of agent data from one env step() call. .. testcode:: import numpy as np ac = AgentConnectorDataType( env_id="env_1", agent_id=None, data={ "agent_1": np.array([1, 2, 3]), "agent_2": np.array([4, 5, 6]), } ) Or a single agent data ready to be preprocessed. .. testcode:: ac = AgentConnectorDataType( env_id="env_1", agent_id="agent_1", data=np.array([1, 2, 3]), ) etc.

bveeramani · 2023-07-18T23:07:49Z

rllib/connectors/connector.py

+    .. testcode::
+        from ray.rllib.connectors.action.lambdas import (


Need newline here

bveeramani · 2023-07-18T23:08:25Z

rllib/core/learner/learner.py

+            # We use PPO and torch as an example here because many of the showcased
+            # components need implementations to come together. However, the same
+            # pattern is generally applicable.


This might be more readable if you move it out of the testcode as plaintext

bveeramani · 2023-07-18T23:10:17Z

rllib/core/learner/learner.py

-        # remove a module
-        learner.remove_module("new_player")
+            # Take one gradient update on the module and report the results
+            # results = learner.update(...)


(Here and below) What would happen if we uncommented this and replaced the ellipses with an actual argument?

We would need to construct a multi-agent training batch here.
This would be a nested dict with quite a lot of KV pairs with all values being torch tensors.
It's just too bug of a thing to construct here. We need to be able to generate example batches some time in the future but we are not there today.

bveeramani · 2023-07-18T23:16:59Z

rllib/core/models/specs/checker.py

+
+        model = MyModel()
+        model.forward({"obs": torch.randn(32, 64)}) # No error
+        model.forward({"obs": torch.randn(32, 32)}) # raises ValueError


This will cause CI to fail. You could put this in a subsequent testcode that's explicitly skipped

model.forward({"obs": torch.randn(32, 64)}) # No error ... and this example raises a ValueError .. testcode:: :skipif: True model.forward({"obs": torch.randn(32, 32)}) .. testoutput:: Traceback: ...

bveeramani · 2023-07-18T23:18:04Z

rllib/core/rl_module/rl_module.py


-    .. code-block:: python
+        # Example for creating a sampling loop:


This example is long. For better readability, it might be better to split this into separate testcodes (one for sampling, training, inference, etc)

bveeramani · 2023-07-18T23:18:32Z

rllib/models/distributions.py

@@ -13,10 +13,24 @@ class Distribution(abc.ABC):
    """The base class for distribution over a random variable.


This file explicitly skipped in the BUILD file. Could we remove it from the list?

bveeramani · 2023-07-18T23:18:52Z

rllib/models/distributions.py

-        >>> action_logits = model.forward(obs)
-        >>> action_dist = Distribution(action_logits)
+
+    .. doctest::


This directive is unecessary

bveeramani · 2023-07-18T23:19:09Z

rllib/models/distributions.py

+        .. doctest::
+
+            >>> import numpy as np
+            >>> from ray.rllib.models.distributions import Distribution
+
+            >>> class Uniform(Distribution):
+            ...    def __init__(self, lower, upper):
+            ...        self.lower = lower
+            ...        self.upper = upper
+            ...
+            ...    def sample(self):
+            ...        return self.lower + (self.upper - self.lower) * np.random.rand()
+            ...
+            ...    def logp(self, x):
+            ...        ...
+            ...
+            ...    def kl(self, other):
+            ...        ...
+            ...
+            ...    def entropy(self):
+            ...        ...
+            ...
+            ...    @staticmethod
+            ...    def required_input_dim(space):
+            ...        ...
+            ...
+            ...    def rsample(self):
+            ...        ...
+            ...
+            ...    @classmethod
+            ...    def from_logits(cls, logits, **kwargs):
+            ...        return Uniform(logits[:, 0], logits[:, 1])
+
+            >>> logits = np.array([[0.0, 1.0], [2.0, 3.0]])
+            >>> my_dist = Uniform.from_logits(logits)
+            >>> sample = my_dist.sample()


This example is longer, so it'd be more readable as testcode

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com> Signed-off-by: NripeshN <nn2012@hw.ac.uk>

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com> Signed-off-by: Victor <vctr.y.m@example.com>

ArturNiederfahrenhorst added 2 commits July 17, 2023 16:04

initial

8f026cd

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

learners and model configs

c734f16

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

ArturNiederfahrenhorst assigned kouroshHakha Jul 17, 2023

ArturNiederfahrenhorst requested review from sven1977, gjoliver, avnishn, smorad, maxpumperla, kouroshHakha and krfricke as code owners July 17, 2023 23:26

ArturNiederfahrenhorst added 3 commits July 17, 2023 17:03

Add checker and add tensorspec TypeSpec representation

0425cae

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

Add rl_module.py

dc05d7c

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

Add distributions

94037cd

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

ArturNiederfahrenhorst commented Jul 18, 2023

View reviewed changes

ArturNiederfahrenhorst added 2 commits July 17, 2023 17:57

Turn some example into testcode examples

8dcc5d7

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

Turn other ones into testcode

503d9ff

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

kouroshHakha approved these changes Jul 18, 2023

View reviewed changes

Remove useless connector part to trigger doctests

6fa3c85

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

ArturNiederfahrenhorst requested a review from a team as a code owner July 18, 2023 16:24

ArturNiederfahrenhorst added 2 commits July 18, 2023 10:48

Merge branch 'master' into doctests

fe1c3ce

Merge branch 'master' into doctests

0a669e4

kouroshHakha merged commit 1ff3b1d into ray-project:master Jul 18, 2023

bveeramani reviewed Jul 18, 2023

View reviewed changes

ArturNiederfahrenhorst mentioned this pull request Jul 19, 2023

[RLlib] Update docs examples inside core folder (again) #37558

Merged

Bhav00 pushed a commit to Bhav00/ray that referenced this pull request Jul 28, 2023

[RLlib] Turn doc tests into '.. doctest::' (ray-project#37492)

0ebda1f

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com>

NripeshN pushed a commit to NripeshN/ray that referenced this pull request Aug 15, 2023

[RLlib] Turn doc tests into '.. doctest::' (ray-project#37492)

f9657b0

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com> Signed-off-by: NripeshN <nn2012@hw.ac.uk>

arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023

[RLlib] Turn doc tests into '.. doctest::' (ray-project#37492)

a25fdcd

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023

[RLlib] Turn doc tests into '.. doctest::' (ray-project#37492)

2e4bf56

Signed-off-by: Artur Niederfahrenhorst <attaismyname@googlemail.com> Signed-off-by: Victor <vctr.y.m@example.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RLlib] Turn doc tests into '.. doctest::' #37492

[RLlib] Turn doc tests into '.. doctest::' #37492

ArturNiederfahrenhorst commented Jul 17, 2023

ArturNiederfahrenhorst Jul 18, 2023

bveeramani Jul 18, 2023

bveeramani Jul 18, 2023

bveeramani Jul 18, 2023

bveeramani Jul 18, 2023

bveeramani Jul 18, 2023

ArturNiederfahrenhorst Jul 19, 2023

bveeramani Jul 18, 2023

bveeramani Jul 18, 2023

bveeramani Jul 18, 2023

bveeramani Jul 18, 2023

bveeramani Jul 18, 2023

		.. testcode::
		from ray.rllib.connectors.action.lambdas import (


		.. code-block:: python
		# Example for creating a sampling loop:

		@@ -13,10 +13,24 @@ class Distribution(abc.ABC):
		"""The base class for distribution over a random variable.

[RLlib] Turn doc tests into '.. doctest::' #37492

[RLlib] Turn doc tests into '.. doctest::' #37492

Conversation

ArturNiederfahrenhorst commented Jul 17, 2023

Why are these changes needed?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment