[rllib] Improve test_single_agent_env_runner to prevent flaky tests
#58397
+168
−33
Conversation
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
Signed-off-by: Mark Towers <[email protected]>
pseudo-rnd-thoughts
added
rllib
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves the tests for SingleAgentEnvRunner.sample to prevent flakiness and updates the documentation to reflect the correct behavior. The changes are generally good and address the stated goals. I've identified a couple of areas in the tests where the assertions could be more precise to better validate the functionality. My review includes suggestions to tighten these assertions.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: Mark Towers <[email protected]>
pseudo-rnd-thoughts
changed the title
test_single_agent_env_runner to prevent flaky teststest_single_agent_env_runner to prevent flaky tests
Signed-off-by: Mark Towers <[email protected]>
simonsays1980
approved these changes
Nov 14, 2025
Contributor
simonsays1980
left a comment
simonsays1980
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the refinement @pseudo-rnd-thoughts !
|
|
||
| # Sample n timesteps. | ||
| if num_timesteps is not None: | ||
| assert num_timesteps >= 0 |
| num_timesteps=10, num_episodes=10, random_actions=True | ||
| ), | ||
| ) | ||
| # Verify that an error is raised if a negative number is used |
| env_runner.stop() | ||
|
|
||
| def test_env_context(self): | ||
| """Tests, whether SingleAgentEnvRunner can pass kwargs to the environments correctly.""" |
Contributor
There was a problem hiding this comment.
Great! Thanks for adding this. This was missing but is very important.
simonsays1980
pushed a commit
that referenced
this pull request
Nov 17, 2025
…_env` (#58410) ## Description Allow users to use environments that are already vectorized for `SingleAgentEnvRunner` With `gymnasium.make_vec`, users have the option to either use the `SyncVectorEnv` to vectorize a base environment or to directly create a vector environment using the `vectorize_mode: gymnasium.VectorizeMode`. This PR utilises the `env_runners(gym_env_vectorize_mode=...)` argument to support `VectorizeMode.VECTOR_ENTRY_POINT` ``` import gymnasium as gym config = ... config.env_runners( gym_env_vectorize_mode=gym.VectorizeMode.VECTOR_ENTRY_POINT, ) ``` An important change related to this PR is that the values accepted for the vectorize mode is either the enum (`VectorizedMode.ASYNC`, etc) or the enum values (`"async"`, etc) as before it was the string version was the enum name (`"ASYNC"`) rather than the enum value itself. ## Related issues Completion of #57643, ## Additional information #58397 must be merged first We should apply a similar change to the `MultiAgentEnvRunner.make_env` --------- Signed-off-by: Mark Towers <[email protected]> Co-authored-by: Mark Towers <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
In improving the
SingleEnvRunner.make_env, I found that some of the tests could be flaky.This PR improves the testing, in particular, to
sampleto ensure that the tests don't fail occasionally and the documentation to reflect this.The primary flaky problem I found is that
sample(num_timesteps=X)will not always return a total ofXtimesteps, rather at least X timesteps up to the number of environments more.I'm updated the documentation to clarify this for users.
In addition, I've added tests for when neither the number of timesteps or episodes are given and for the
force_resetargument