Fix weight normalization inside prioritized experience replay - #570
Conversation
They fails when normalize_by_max == 'batch' for now, which will be fixed later.
This bug implies that, before this commit, normalize_by_max == 'batch' functioned just the same as normalize_by_max == 'memory'.
|
The test seems to be failing on Python 2 |
|
/test |
|
Successfully created a job for commit 7c72643: |
|
Now CI passed. |
marioyc
left a comment
marioyc
left a comment
There was a problem hiding this comment.
LGTM, would it be necessary to evaluate how this affects some of the agents? like Rainbow for example
|
We need to re-evaluate agents, but since the current behavior is clearly wrong, I think it is good to merge it now. By specifying |
|
Perhaps we could modify existing reproduction scripts with |
|
I guess that would do as a fix, would that need to be part of this PR? |
That sounds good. I will add that change to this PR. |
so that the script still reproduces the current benchmark results.
|
/test |
|
Successfully created a job for commit f20b865: |
|
@marioyc Can you check the change I made and merge if it's ok? |
5 participants
This PR fixes the wrong computation of
min_probabilityinPrioritizedReplayBufferwhennormalize_by_max == 'batch', which is the default behavior. The previous behavior was unexpectedly the same asnormalize_by_max == 'memory'.