Polish: seeded rollout-buffer RNG and eval-freeze for the normalization wrappers - #22
Open
DenisDrobyshev wants to merge 2 commits into
Open
Polish: seeded rollout-buffer RNG and eval-freeze for the normalization wrappers#22DenisDrobyshev wants to merge 2 commits into
DenisDrobyshev wants to merge 2 commits into
Conversation
The on-policy rollout buffer shuffled minibatches with the global numpy RNG, so two on-policy agents constructed with different seeds in the same process shared (and perturbed) one shuffle stream, and the buffer did not actually own a seedable RNG the way the README claims every buffer does. Give it its own numpy Generator seeded from the agent's seed (threaded through the on-policy base and IPPO). PPO is now reproducible per seed and isolated from the global RNG state; a regression test covers seeded, isolated, and reordering behaviour.
NormalizeObservation and NormalizeReward updated their running statistics on every step with no way to stop, so using them for evaluation kept moving the statistics toward the evaluation distribution and reported reward-normalized returns instead of real ones. Both now take set_training(bool): with training frozen, the observation wrapper normalizes with the statistics it learned during training, and the reward wrapper passes the original rewards through untouched. Matches the freeze semantics of Stable-Baselines3 VecNormalize. Tests and docs updated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First batch from a correctness pass over the library. The core is in good shape: I went
through the on-policy and off-policy training loops, the GAE and n-step return
computations, SAC, TD3, and IMPALA's V-trace, plus
evaluate_policyand the rliable-styleevaluation statistics (IQM, bootstrap CI, probability of improvement), and found them
correct. The two fixes here are both in the utility layer.
Seeded rollout-buffer RNG
RolloutBuffershuffled its minibatches with the global NumPy RNG. Two on-policy agentsconstructed with different seeds in the same process therefore shared one shuffle stream
and perturbed each other, and the buffer never actually held the per-instance RNG the
README credits every buffer with. It now owns a
numpy.random.Generatorseeded from theagent's seed, threaded through the on-policy base and IPPO. PPO is reproducible per seed
and independent of global RNG state; a regression test covers the seeded, isolated, and
reordering cases.
Eval-freeze for the normalization wrappers
NormalizeObservationandNormalizeRewardupdated their running statistics on every stepwith no way to stop, so using them during evaluation kept the statistics drifting toward
the evaluation distribution and reported reward-normalized returns instead of real ones.
Both now take
set_training(bool): frozen, the observation wrapper normalizes with thestatistics learned during training and the reward wrapper passes the environment's original
rewards through untouched. This matches the freeze semantics of Stable-Baselines3's
VecNormalize.Tests, docs, and the CHANGELOG are updated. ruff, mypy, and the affected tests pass locally.