[Bug]: DummyVecEnv.seed() resets the environment

[Kallinteris-Andreas]

🐛 Bug

Seeding a DummyVecEnv resets the env with that seed (instead of saving the seed to be used in the next reset)

stable-baselines3/stable_baselines3/common/vec_env/dummy_vec_env.py

Line 73 in d6ddee9

def seed(self, seed: Optional[int] = None) -> List[Union[None, int]]:

stable-baselines3/stable_baselines3/common/utils.py

Line 563 in d6ddee9

env.reset(seed=seed)

This behavior is weird and undocumented, and may cause reproducibility issues, I am assuming this happened in the transition to gymnasium-v26 API

the expected behavior would be to save the seed to be used in the next reset

Also, DummyVecEnv.seed() returns a List[None] instead of None

Possible Solutions:

1. save the seed to be used in the next DummyVecEnv.reset()
2. add the seed argument to reset (Just as Gymansium.Env has done and remove the seed() function)

As a Gymnasium dev, I would recommend the second solution

Note: the same behavior can be seen in subproc_vec_env

To Reproduce

import gymnasium

from stable_baselines3.common.vec_env import DummyVecEnv

MY_SEED = 0

env = gymnasium.make('Ant-v4')
d_env = DummyVecEnv([lambda: gymnasium.make('Ant-v4')])

obs, _ = env.reset(seed=MY_SEED)
obs1, _ = env.reset()
seeds = d_env.seed(MY_SEED)
d_obs = d_env.reset()

assert (d_obs[0] == obs1).all()

breakpoint()

Relevant log output / Error message

No response

System Info

$ pip list | grep stabl
stable-baselines3 2.0.0a5

$ python --version
Python 3.10.10

$ pip list | grep torch
torch 1.12.1

$ pip list | grep gym
gymnasium 0.28.1

Checklist

• I have checked that there is no similar issue in the repo
• I have read the documentation
• I have provided a minimal working example to reproduce the bug
• I've used the markdown code blocks for both code and stack traces.

[qgallouedec]

In addition, from the documentation:

• the reset() method doesn't take any parameter. If you want to seed the pseudo-random generator, you should call vec_env.seed(seed=seed) and obs = vec_env.reset() afterward.

This means that the environment is actually reset twice. I agree that this is not very intuitive.

[araffin]

Hello,

I am assuming this happened in the transition to gymnasium-v26 API

This happened indeed when Gym broke its API.

save the seed to be used in the next DummyVecEnv.reset()

This is actually a nicer option I didn't consider. I would be happy to receive a PR =)
(we would need to document the behavior though)
We still need to double check but I think we always call reset() after calling seed(), we could also remove the compatibility wrapper I had.

add the seed argument to reset (Just as Gymansium.Env has done and remove the seed() function)

I don't think it's needed and it would change/break too many things (all the VecEnvWrapper for instance).

[Kallinteris-Andreas]

I don't think it's needed and it would change/break too many things (all the VecEnvWrapper for instance).

This would be the best long term option in my opinion.
There is no functional benefit in keeping the seed function over adding a seed argument.
(though you probably know the project better than me)

I can make a PR, all I need is conformation that the only affected classes (ones with a seed() function) are subproc_vec_env and DummyVecEnv

[araffin]

There is no functional benefit in keeping the seed function over adding a seed argument.

Consistency over time/long-term support is the main benefit.
Both options are fine, but one keeps the API (and behavior) the same from a user perspective, no matter which version of gym/gymnasium/SB3 is used.

SB3 is no longer a new library, we should avoid breaking people's code when it is not needed.
This is something that Gym didn't follow and caused a lot of unnecessary effort for users. (sail-sg/envpool#240 (comment)).

I can make a PR, all I need is conformation that the only affected files/classes are subproc_vec_env and DummyVecEnv

yes and the async version in SB3 contrib.

PS: I already spent too much time discussing that issue in gym: openai/gym#2422 (comment)