Baselines: Gamma in VecNormalize for rms updates.

Created on 24 Aug 2018  Â·  9Comments  Â·  Source: openai/baselines

I have observed that the VecNormalize class updates the reward running mean statistics with what looks to me like a discounted reward:

def step_wait(self):
    obs, rews, news, infos = self.venv.step_wait()
    self.ret = self.ret * self.gamma + rews # here
    obs = self._obfilt(obs)
    if self.ret_rms:
        self.ret_rms.update(self.ret)
        rews = np.clip(rews / np.sqrt(self.ret_rms.var + self.epsilon), -self.cliprew, self.cliprew)
    return obs, rews, news, infos

I can't see why this helps at all, I would directly use rews in the rms update. Am I missing something here?

Thanks.

question

Most helpful comment

Regarding 1), the gamma corresponds to the discount factor used in the RL algorithm. We want to normalize rewards so that the advantages in the RL algorithm have a nice magnitude, meaning that we should use the same gamma that's used to compute the advantages. The returns are being averaged over the entire course of training by ret_rms, so the gamma doesn't directly effect how fast our normalization coefficient is changing.

Regarding 2), subtracting the mean from the rewards would change the dynamics of the environment. For example, if your rewards are all 1, that is very different than if your rewards are all 0. In the former case, the agent wants to live; in the latter, it doesn't care if it lives or dies. The reward clipping looks like a bit of a hack, and there's probably no way to do it truly "correctly". Centering the clip range at the mean reward wouldn't quite make sense, I don't think, and I can't think of a perfect way to do it.

All 9 comments

it looks like the statistics are collected for episode returns (not for rewards), but I don't really know what's benefit of using that for the reward normalization vs using stats of the rewards themselves. Also, in that case, there may be a bug where you'd need to reset self.ret to 0 at the beginning of the episode, here: https://github.com/openai/baselines/blob/92b9a3725710a20ec62120e7b1533afc84a3df27/baselines/common/vec_env/vec_normalize.py#L39. @joschu @unixpickle could you comment please?

Normalization is important because neural networks trained with Adam (and most other algorithms, as well) don't learn well with very large or very small targets. Returns are what we really want to normalize, because they are ultimately what drive the value function and policy.

To see why normalizing returns is different than normalizing rewards, take the simple case where the rewards are large but cancel each other out. For example, the rewards might be [100, -101, 102, -99, ...]. In this case, the returns will be fairly small, even though the rewards are large. Thus, the advantages we feed into the RL algorithm will already be fairly well-behaved. In a different environment, the rewards might be [10, 10, 10, 10, ...]. In this case, the rewards are not that large, but the returns are likely to be on the order of 1000 if we use gamma=0.99.

thanks @unixpickle! Do you agree with resetting returns to 0 in reset method, or is there some reason why we should not?

I agree it makes sense to normalize returns. The current method effectively
applies a sliding window. I do have two concerns:

1) with gamma=0.99 this window is effectively of length ~100 which may be
less than a single episode, so the effect of normalizing across episodes is
lost.

2) we are dividing by std without subtracting mean. But the result is
clipped symmetrically around 0.

My intuition is that we want to average across a much larger window and
also subtract the mean. Have I missed something?

Andy

atwigg.com

On Mon, Aug 27, 2018, 5:49 PM pzhokhov notifications@github.com wrote:

thanks @unixpickle https://github.com/unixpickle! Do you agree with
resetting returns to 0 in reset method, or is there some reason why we
should not?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/openai/baselines/issues/538#issuecomment-416290557,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACXttWheR8t8zH0uq5i1FJ9NIbkDPQoAks5uVCMGgaJpZM4WK6Q3
.

Regarding 1), the gamma corresponds to the discount factor used in the RL algorithm. We want to normalize rewards so that the advantages in the RL algorithm have a nice magnitude, meaning that we should use the same gamma that's used to compute the advantages. The returns are being averaged over the entire course of training by ret_rms, so the gamma doesn't directly effect how fast our normalization coefficient is changing.

Regarding 2), subtracting the mean from the rewards would change the dynamics of the environment. For example, if your rewards are all 1, that is very different than if your rewards are all 0. In the former case, the agent wants to live; in the latter, it doesn't care if it lives or dies. The reward clipping looks like a bit of a hack, and there's probably no way to do it truly "correctly". Centering the clip range at the mean reward wouldn't quite make sense, I don't think, and I can't think of a perfect way to do it.

>

Regarding 1), the gamma corresponds to the discount factor used in the RL
algorithm.

I see. So we want the discounted rewards computed in the RL algorithm to
have unit variance. I guess there is a small concern that the RL algorithm
discounts 'backwards' and this method discounts in the opposite direction,
but perhaps in practise this isn't too much of an issue?
In this case, I assume we need to set ret=0 in reset() as per
@pzhokhov's comment?

Thanks @pzhokhov, @unixpickle, @andytwigg, lets see if I got this well enough:

  • VecNormalize's gamma should match the discount factor of the RL algorithm.
  • This is because the code needs to keep track of per env. returns.
  • Returns running avg./var. stats are global, not per env.
  • step_wait computes the "normalized reward" for the step with the returns running stats.
  • Finally reward is clipped symmetrically around 0.

If I'm correct I guess the code just needs to update self.ret to 0 acordingly, not just in reset but also in step_wait (in case of terminal/absorbing state). This won't reset running avg./var. returns, only per env returns, so returns from a previous episode won't leak into the next one causing the first steps to be a bit off.

Regards.

If I'm correct I guess the code just needs to update self.ret to 0
acordingly, not just in reset but also in step_wait (in case of
terminal/absorbing state).

Yes, that's a good point.

@pzhokhov this looks good now, can the PR be merged?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

PolarisYxh picture PolarisYxh  Â·  10Comments

HitLyn picture HitLyn  Â·  9Comments

astier picture astier  Â·  8Comments

wonchul-kim picture wonchul-kim  Â·  7Comments

Nara0731 picture Nara0731  Â·  19Comments