🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/2047

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 5 New Failures, 16 Unrelated Failures

As of commit 7b5785d with merge base 09c934d ():

This comment was automatically generated by Dr. CI and updates every 15 minutes.

I'd rather have them as a separate PR if possible

This would however be a synchronous environment so the tutorial would NOT SHOW:

Moved the changes to #2048

@vmoens this is ready for review

@elliottower @coderSupreme789 here is the first draft of the tuto in case you are curious and want to have a look.

Thanks for this, very complete!

I'm wondering if it isn't a bit too long. The code is about 1k lines of code long, there's a lot of theory and code. When someone goes through this tuto, what should they get out of it exactly (a theory about MARL, a way to code DDPG in MARL, how to use the env?) Can't this be split in more tutos? Or can we remove some extra bits that aren't 100% required to train the algo?

What the tuto is doing is showing the env and the grouping mechanism and training it.

• Regarding the code: All the python code is necessary for training, except the section where i print the env specs and rollout which i think is needed to show readers the agent groups
• Regarding the text: this follows the PPO and multi-agent PPO format. I can remove some theory, this was there as it motivates some choices on how to centralize the critic and so on, but if you think too verbous we can cut. I can also cut the DDPG anf off-policy explainations

I should have access to S3 soon, bear with me

EDIT: feel free to send me the files via PM and I'll store them on S3

• Regarding the text: this follows the PPO and multi-agent PPO format. I can remove some theory, this was there as it motivates some choices on how to centralize the critic and so on, but if you think too verbous we can cut. I can also cut the DDPG anf off-policy explainations

I guessed so, but most feedback I got even from very close, top collaborators such as yourself is that our tutorials are very long and hard to follow, if not worse than that.
If we can make sure that tutorials that are aimed at a large audience (which I suspect is the case here since it starts with the very basics like off-policy vs on-policy) are also completed in 10 mins top I think they will be more impactful.

Concretely, I would either make this tutorial advanced and presuppose that users have an extensive knowledge of MARL and RL in general, and just point them to refs in case context is needed, or split the tutorial in 2-5 sub-tutorials

• Regarding the text: this follows the PPO and multi-agent PPO format. I can remove some theory, this was there as it motivates some choices on how to centralize the critic and so on, but if you think too verbous we can cut. I can also cut the DDPG anf off-policy explainations

I guessed so, but most feedback I got even from very close, top collaborators such as yourself is that our tutorials are very long and hard to follow, if not worse than that. If we can make sure that tutorials that are aimed at a large audience (which I suspect is the case here since it starts with the very basics like off-policy vs on-policy) are also completed in 10 mins top I think they will be more impactful.

Concretely, I would either make this tutorial advanced and presuppose that users have an extensive knowledge of MARL and RL in general, and just point them to refs in case context is needed, or split the tutorial in 2-5 sub-tutorials

I have removed basically all the RL background intro from the tutorial and am making it advanced since it is kind of a more complex case than the PPO one

Can we unify that? If not what's the blocker?

I answered that in the thread. We want to keep those separate to allow different choices across groups. Plus losses cannot physicall work with multiple groups

Got it!
Not super clean that we ask people to pass _reset, which is private
We should ask for done instead

The problem is that the dones are still one per group plus the shared one. The _reset on the other hand is just one

Got it!
Not super clean that we ask people to pass _reset, which is private
We should ask for done instead

The problem is that the dones are still one per group plus the shared one. The _reset on the other hand is just one

IIRC the global reset/done precedes over the local ones.

Am I right that what you're trying to do is override the local resets with the global one then?
Couldn't we use "done" like you're using reset?

Can we unify that? If not what's the blocker?

I answered that in the thread. We want to keep those separate to allow different choices across groups. Plus losses cannot physicall work with multiple groups

Sorry I missed this, the thread must have been closed.

I don't understand this statement though, can you elaborate?
In particular

losses cannot physicall work with multiple groups

what does "physicall" mean here?
What is blocking us to use a single loss across groups?

We want to keep those separate to allow different choices across groups

What "choices"? Hyperparameters?
What I see is that they are all the same so unless we use different ones it isn't immediate to understand.

Am I right that what you're trying to do is override the local resets with the global one then? Couldn't we use "done" like you're using reset?

So in pettingzoo we have the global done and the local dones but only the global _reset. So the local resets just do not exist.

since the class needs a reset for each agent key, we are telling all reward keys to track the global reset.

Can we unify that? If not what's the blocker?

I answered that in the thread. We want to keep those separate to allow different choices across groups. Plus losses cannot physicall work with multiple groups

Sorry I missed this, the thread must have been closed.

I don't understand this statement though, can you elaborate? In particular

losses cannot physicall work with multiple groups

what does "physicall" mean here? What is blocking us to use a single loss across groups?

We want to keep those separate to allow different choices across groups

What "choices"? Hyperparameters? What I see is that they are all the same so unless we use different ones it isn't immediate to understand.

Ok let me elaborate more, because this is quite a fundamental concept about the whole MARL api and BenchMARL.

• losses are stateful components that only take one set of keys (reward,state,done, and so on). Therefore they cannot operate across multiple groups
• each group can have different global values, like global critic output, that sit at root. Therefore if you kept multiple groups in the same loss it is unclear how to handle these
• in the example of this tutorial, we stop training one group after x iterations. This would not be possible with a shared buffer/loss/optimizer

The core of the MARL API is that if you want a shared loss/buffer/optimizer you can keep agents in the same group. But when agents are in different groups they can be decoupled into individual components so that you can

• use different losses/buffers(/optimizers for each group
• Train them asynchronously
• not pollute the data of other groups when writing global keys for your group

Thanks for the clarification
I can see why it isn't easy to have one loss for multiple groups atm, but don't we want that to be a feature, rather than asking users to repeat the tedious construction of N different buffers, N different optimizers and N different losses?
It feels like there's a lot of boilerplate that we could get rid of without too much trouble from within the library.

Since we can use param_groups in the optimizers, we could obtain the same behaviour as having many optimizers by just splitting the param groups carefully. Note that this will come with significant speedups on cuda too since you'll be able to update your params asynchronously.

Train them asynchronously

Isn't the training synchronous here? They're trained sequentially for a given number of steps no?

in the example of this tutorial, we stop training one group after x iterations. This would not be possible with a shared buffer/loss/optimizer

Can you point me to the part of the tutorial that does that? What I see is that there's a constant number of steps for each component and the learning rates as well as other hyperparameters are all shared. From a non MARL trained eye it seems like there's a bunch of stuff that could be simplified (for instance why not having one replay buffer from which you sample and then get the relevant keys with indexing / select?)

Can you point me to the part of the tutorial that does that? What I see is that there's a constant number of steps for each component and the learning rates as well as other hyperparameters are all shared. From a non MARL trained eye it seems like there's a bunch of stuff that could be simplified (for instance why not having one replay buffer from which you sample and then get the relevant keys with indexing / select?)

  # Stop training a certain group when a condition is met (e.g., number of training iterations)
    if iteration == iteration_when_stop_training_evaders:
        del train_group_map["agent"]

Again, at some point we may sample more from the buffer of one group or stop completely sampling from one group like in this tuto.

Isn't the training synchronous here? They're trained sequentially for a given number of steps no?

I mean train them at different times. In this tuto for the seond half of the iterations we are trianing just one

I can see why it isn't easy to have one loss for multiple groups atm, but don't we want that to be a feature, rather than asking users to repeat the tedious construction of N different buffers, N different optimizers and N different losses?
It feels like there's a lot of boilerplate that we could get rid of without too much trouble from within the library.

I doi not agree. I think that this is not a feature we want as it is good to have the clear separation and have losses being single group.

Repeating the construction in a for loop is peanuts compared to the complexity that we would introduce by moving the for loop inside the losses (because at the end of the day the for loop will be somewhere since it is all heterogeneous networks).

I do not think that the current way is boiler plate as users can just experiment with setting different values for groups.

• What if they want a different learning rate for the evaders?
• What if they want MADDPG for chasers and MAPPO for evaders? (at this point one will have a buffer and the other no)
• What if they want to use a curriculum of evaders to train the chasers?

All these cases benefit from the flexibility of training groups independently and I currently see no benefits in making groups share training components (because the for loops will be just nested inside the loss)

But since this is maybe the most important MARL choice of the lib and kind of the core of my internship let's maybe discuss it more also offline