From 9d463bc476bd4b3b7a4ab901298eeb25af385fe8 Mon Sep 17 00:00:00 2001 From: Stefan Heid Date: Mon, 2 Nov 2020 11:45:08 +0100 Subject: [PATCH] Small docstring improvements related to the notion of Rollout (#206) * Small docstring improvements related to the notion of Rollout * documented changes in changelog.rst, added myself to contributers * Minor edits Co-authored-by: Stefan Heid Co-authored-by: Antonin RAFFIN --- docs/misc/changelog.rst | 3 ++- stable_baselines3/common/buffers.py | 9 +++++++++ stable_baselines3/common/off_policy_algorithm.py | 2 +- stable_baselines3/common/on_policy_algorithm.py | 4 +++- stable_baselines3/dqn/dqn.py | 2 +- stable_baselines3/ppo/ppo.py | 5 ++--- tests/test_cnn.py | 2 +- 7 files changed, 19 insertions(+), 8 deletions(-) diff --git a/docs/misc/changelog.rst b/docs/misc/changelog.rst index 60d92bc..000e6e5 100644 --- a/docs/misc/changelog.rst +++ b/docs/misc/changelog.rst @@ -26,6 +26,7 @@ Others: Documentation: ^^^^^^^^^^^^^^ - Updated algorithm table +- Minor docstring improvements regarding rollout (@stheid) Pre-Release 0.10.0 (2020-10-28) @@ -497,7 +498,7 @@ And all the contributors: @EliasHasle @mrakgr @Bleyddyn @antoine-galataud @junhyeokahn @AdamGleave @keshaviyengar @tperol @XMaster96 @kantneel @Pastafarianist @GerardMaggiolino @PatrickWalter214 @yutingsz @sc420 @Aaahh @billtubbs @Miffyli @dwiel @miguelrass @qxcv @jaberkow @eavelardev @ruifeng96150 @pedrohbtp @srivatsankrishnan @evilsocket -@MarvineGothic @jdossgollin @SyllogismRXS @rusu24edward @jbulow @Antymon @seheevic @justinkterry @edbeeching +@MarvineGothic @jdossgollin @stheid @SyllogismRXS @rusu24edward @jbulow @Antymon @seheevic @justinkterry @edbeeching @flodorner @KuKuXia @NeoExtended @PartiallyTyped @mmcenta @richardwu @kinalmehta @rolandgvc @tkelestemur @mloo3 @tirafesi @blurLake @koulakis @joeljosephjin @shwang @rk37 @andyshih12 @RaphaelWag @xicocaio @diditforlulz273 @liorcohen5 @ManifoldFR @mloo3 @SwamyDev @wmmc88 @megan-klaiber diff --git a/stable_baselines3/common/buffers.py b/stable_baselines3/common/buffers.py index 83e6554..8dbce4c 100644 --- a/stable_baselines3/common/buffers.py +++ b/stable_baselines3/common/buffers.py @@ -259,6 +259,15 @@ class ReplayBuffer(BaseBuffer): class RolloutBuffer(BaseBuffer): """ Rollout buffer used in on-policy algorithms like A2C/PPO. + It corresponds to ``buffer_size`` transitions collected + using the current policy. + This experience will be discarded after the policy update. + In order to use PPO objective, we also store the current value of each state + and the log probability of each taken action. + + The term rollout here refers to the model-free notion and should not + be used with the concept of rollout used in model-based RL or planning. + Hence, it is only involved in policy and value function training but not action selection. :param buffer_size: Max number of element in the buffer :param observation_space: Observation space diff --git a/stable_baselines3/common/off_policy_algorithm.py b/stable_baselines3/common/off_policy_algorithm.py index e3ffeb6..a536417 100644 --- a/stable_baselines3/common/off_policy_algorithm.py +++ b/stable_baselines3/common/off_policy_algorithm.py @@ -363,7 +363,7 @@ class OffPolicyAlgorithm(BaseAlgorithm): log_interval: Optional[int] = None, ) -> RolloutReturn: """ - Collect experiences and store them into a ReplayBuffer. + Collect experiences and store them into a ``ReplayBuffer``. :param env: The training environment :param callback: Callback that will be called at each step diff --git a/stable_baselines3/common/on_policy_algorithm.py b/stable_baselines3/common/on_policy_algorithm.py index 4f2ef63..6cd5dbf 100644 --- a/stable_baselines3/common/on_policy_algorithm.py +++ b/stable_baselines3/common/on_policy_algorithm.py @@ -124,7 +124,9 @@ class OnPolicyAlgorithm(BaseAlgorithm): self, env: VecEnv, callback: BaseCallback, rollout_buffer: RolloutBuffer, n_rollout_steps: int ) -> bool: """ - Collect rollouts using the current policy and fill a `RolloutBuffer`. + Collect experiences using the current policy and fill a ``RolloutBuffer``. + The term rollout here refers to the model-free notion and should not + be used with the concept of rollout used in model-based RL or planning. :param env: The training environment :param callback: Callback that will be called at each step diff --git a/stable_baselines3/dqn/dqn.py b/stable_baselines3/dqn/dqn.py index 6d7a455..7a35566 100644 --- a/stable_baselines3/dqn/dqn.py +++ b/stable_baselines3/dqn/dqn.py @@ -135,7 +135,7 @@ class DQN(OffPolicyAlgorithm): def _on_step(self) -> None: """ Update the exploration rate and target network if needed. - This method is called in ``collect_rollout()`` after each step in the environment. + This method is called in ``collect_rollouts()`` after each step in the environment. """ if self.num_timesteps % self.target_update_interval == 0: polyak_update(self.q_net.parameters(), self.q_net_target.parameters(), self.tau) diff --git a/stable_baselines3/ppo/ppo.py b/stable_baselines3/ppo/ppo.py index 47420cf..4ea6253 100644 --- a/stable_baselines3/ppo/ppo.py +++ b/stable_baselines3/ppo/ppo.py @@ -132,8 +132,7 @@ class PPO(OnPolicyAlgorithm): def train(self) -> None: """ - Update policy using the currently gathered - rollout buffer. + Update policy using the currently gathered rollout buffer. """ # Update optimizer learning rate self._update_learning_rate(self.policy.optimizer) @@ -147,7 +146,7 @@ class PPO(OnPolicyAlgorithm): pg_losses, value_losses = [], [] clip_fractions = [] - # train for gradient_steps epochs + # train for n_epochs epochs for epoch in range(self.n_epochs): approx_kl_divs = [] # Do a complete pass on the rollout buffer diff --git a/tests/test_cnn.py b/tests/test_cnn.py index 1c1ef35..ba1040d 100644 --- a/tests/test_cnn.py +++ b/tests/test_cnn.py @@ -154,7 +154,7 @@ def test_features_extractor_target_net(model_class, share_features_extractor): model.lr_schedule = lambda _: 0.0 # Re-activate polyak update model.tau = 0.01 - # Special case for DQN: target net is updated in the `collect_rollout()` + # Special case for DQN: target net is updated in the `collect_rollouts()` # not the `train()` method if model_class == DQN: model.target_update_interval = 1