GRAD_MOMENTUM not used by RMSProp in DQN

[brett-daley]

I noticed that GRAD_MOMENTUM on the following line is never used:

MinAtar/examples/dqn.py

Line 45 in 1918a2f

GRAD_MOMENTUM = 0.95

I think this is likely a bug, since the default momentum for Pytorch's RMSProp is 0 and not 0.95 (see docs).
This PR fixes it by passing the value into the optimizer constructor.

[kenjyoung]

I believe this is actually intentional, although very poorly documented. I'm a bit rusty on the context, but take a look at the original DQN source code here: https://github.com/google-deepmind/dqn/blob/master/dqn/NeuralQLearner.lua. They actually implement something like centred RMSProp without momentum. I believe in the nature version of the DQN paper they have parameters referred to as squared gradient momentum and gradient momentum which are each set to 0.95. Cross referencing this with the source code it seems like these refer to the decay factor on the gradient and the squared gradient which are fixed to be the same in the centred version of the algorithm (see for example the pytorch version here: https://pytorch.org/docs/stable/generated/torch.optim.RMSprop.html).

Nice catch in any case!

[brett-daley]

Ah, I see what you mean! The "momentum" terms are referring to the first and second moment estimations in the denominator. I didn't check the original optimizer before opening the PR 😅

In light of this, I think we could do any of the following:

• Delete the unused variable to avoid confusion,
• Add a comment explaining why it's unused, or
• Just close this PR.

[kenjyoung]

Sorry for the less than expedient reply. Deleting the unused variable makes sense to me! Feel free to amend the PR to do that and I can accept it!

[brett-daley]

@kenjyoung PR updated!

[kenjyoung]

👍

[brett-daley]

@kenjyoung I don't have write access, would you be able to squash and merge for me?

[kenjyoung]

For sure, I thought I did already haha.