Denys88 / rl_games

RL implementations
MIT License
863 stars 146 forks source link

minibatch_size_per_env bug ? #286

Closed Yougo-robotics closed 3 months ago

Yougo-robotics commented 4 months ago

I believe there is a logical flaw when using the "minibatch_size_per_env" config parameter, rendering it useless. Here is the code I will refer to :

a2c_common.py

More specifically this part :

        self.games_num = self.config['minibatch_size'] // self.seq_length # it is used only for current rnn implementation
        self.batch_size = self.horizon_length * self.num_actors * self.num_agents
        self.batch_size_envs = self.horizon_length * self.num_actors

        assert(('minibatch_size_per_env' in self.config) or ('minibatch_size' in self.config))
        self.minibatch_size_per_env = self.config.get('minibatch_size_per_env', 0)
        self.minibatch_size = self.config.get('minibatch_size', self.num_actors * self.minibatch_size_per_env)

        self.num_minibatches = self.batch_size // self.minibatch_size
        assert(self.batch_size % self.minibatch_size == 0)

        self.mini_epochs_num = self.config['mini_epochs']

if your config looks like :

minibatch_size: 256
minibatch_size_per_env: 8  #just random examples

then minibatch_size_per_env will silently be ignored due to the line : self.minibatch_size = self.config.get('minibatch_size', self.num_actors * self.minibatch_size_per_env)

So you might be tempted to remove minibatch_size: 256 and only keep minibatch_size_per_env: 8 in the config file, however the line : self.games_num = self.config['minibatch_size'] will prevent you to do so.

TLDR : Can't use minibatch_size_per_env in the config file.

Denys88 commented 4 months ago

Thanks, I'll doublecheck what happend (for some reason I remember it worked some time ago :D ) and fix it.

Denys88 commented 3 months ago

Should be fixed. Closing it.