Render initial state in evaluate_policy (Fixes #1692)#2257
Open
midhunxavier wants to merge 1 commit into
Open
Conversation
evaluate_policy() only called env.render() after env.step(), so the initial post-reset state was never rendered (a problem for envs with dynamic/random initial states, e.g. via EvalCallback). This renders the initial state once after reset, before the first predict(). The final post-terminal frame is still not rendered because VecEnv auto-resets sub-environments; this caveat is documented in the docstring and via an inline comment, matching the maintainer's accepted minimal approach. Adds a regression test that fails on the old behavior and passes now. AI-assistance disclosure: this change was developed with the assistance of an LLM (Claude), per the SB3 contributing guidelines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Description
evaluate_policy()(used byEvalCallback) only calledenv.render()afterenv.step(), so the initial post-reset state of each episode was never rendered.This adds a single
env.render()afterenv.reset(), before the firstpredict(), so the initial state is shown. The final (post-terminal) frame isintentionally still not rendered because
VecEnvauto-resets sub-environments;this caveat is now documented in the
renderdocstring with a workaround.This implements the minimal approach @araffin proposed in the issue thread (render
before predict + document the final-frame limitation), not the rejected
VecEnv-subclass approach.Motivation and Context
closes #1692
Types of changes
Checklist
docs/misc/changelog.md) (required).make format(required)make check-codestyleandmake lint(required)make pytestandmake typeboth pass. (required)make doc(required)AI-assistance disclosure (per CONTRIBUTING.md): this change was developed with
the assistance of an LLM (Claude). The approach was proposed by the maintainer
(@araffin) in #1692; I have reviewed and own the change.