Skip to content

Polish: seeded rollout-buffer RNG and eval-freeze for the normalization wrappers - #22

Open
DenisDrobyshev wants to merge 2 commits into
mainfrom
polish/audit-fixes
Open

Polish: seeded rollout-buffer RNG and eval-freeze for the normalization wrappers#22
DenisDrobyshev wants to merge 2 commits into
mainfrom
polish/audit-fixes

Conversation

@DenisDrobyshev

Copy link
Copy Markdown
Member

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_policy and the rliable-style
evaluation 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

RolloutBuffer shuffled its minibatches with the global NumPy RNG. Two on-policy agents
constructed 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.Generator seeded from the
agent'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

NormalizeObservation and NormalizeReward updated their running statistics on every step
with 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 the
statistics 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.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant