google / dopamine

Dopamine is a research framework for fast prototyping of reinforcement learning algorithms.
https://github.com/google/dopamine
Apache License 2.0
10.42k stars 1.36k forks source link

Jax agents not passing required argument to replay buffer constructor #200

Closed tylerkastner closed 1 year ago

tylerkastner commented 1 year ago

replay_capacity and batch_size are required arguments for the OutOfGraphReplayBuffer constructor: https://github.com/google/dopamine/blob/a2753dae222c75ae991758d4110a84bc01c3215f/dopamine/replay_memory/circular_replay_buffer.py#L104-L108 However they are not being passed in the _build_replay_buffer method of any Jax agent: https://github.com/google/dopamine/blob/a2753dae222c75ae991758d4110a84bc01c3215f/dopamine/jax/agents/dqn/dqn_agent.py#L350-L357 This appears to only affect Jax agents, non-Jax agents use the WrappedReplayBuffer constructor for which replay_capacity and batch_size are not required arguments.

psc-g commented 1 year ago

hi tyler, these are injected via gin bindings, e.g. here: https://github.com/google/dopamine/blob/master/dopamine/jax/agents/dqn/configs/dqn.gin#L36

if you remove those lines from the gin files you'll see that you will get an error.

On Tue, Oct 11, 2022 at 2:12 PM Tyler Kastner @.***> wrote:

replay_capacity and batch_size are required arguments for the OutOfGraphReplayBuffer constructor:

https://github.com/google/dopamine/blob/a2753dae222c75ae991758d4110a84bc01c3215f/dopamine/replay_memory/circular_replay_buffer.py#L104-L108 However they are not being passed in the _build_replay_buffer method of any Jax agent:

https://github.com/google/dopamine/blob/a2753dae222c75ae991758d4110a84bc01c3215f/dopamine/jax/agents/dqn/dqn_agent.py#L350-L357 This appears to only affect Jax agents, non-Jax agents use the WrappedReplayBuffer constructor for which replay_capacity and batch_size are not required arguments.

— Reply to this email directly, view it on GitHub https://github.com/google/dopamine/issues/200, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE3CCMLII754SJ77BM24CQTWCWUXBANCNFSM6AAAAAARCRI43U . You are receiving this because you are subscribed to this thread.Message ID: @.***>

tylerkastner commented 1 year ago

Ah that does it, thanks!