[Feature] CNN version of MultiAgentMLP

[MarkHaoxiang]

Description

Added a MultiAgentConvNet module, which wraps a ConvNet to work in the multiagent space.

Motivation and Context

This is an utility written for PettingZoo Multiagent environments which have pixel based observations.

Types of changes

What types of changes does your code introduce? Remove all that do not apply:

• Bug fix (non-breaking change which fixes an issue)
• [ x] New feature (non-breaking change which adds core functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (update in the documentation)
• Example (update in the folder of examples)

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

• [ x] I have read the CONTRIBUTION guide (required)
• My change requires a change to the documentation.
• [ x] I have updated the tests accordingly (required for a bug fix or a new feature).
• I have updated the documentation accordingly.

[MarkHaoxiang]

@matteobettini

[matteobettini]

This is incredible work, thanks so much! I was actually in need of a model like this.

I left some comments, but after those are addressed we should be there!

[matteobettini]

also make sure you format the files and lint them.
we have a pre-commit hook, instructiuons on how to run it should be in the CONTRIBUTING file

[MarkHaoxiang]

Thank you for the review. I have updated the changes requested.

[matteobettini]

LGTM thanks for this

@vmoens can you have a pass?

[matteobettini]

@vmoens I’m considering merging this as is and do the refactoring to vmap in a second time as in #1497, would that work for you? Unless @MarkHaoxiang wants to take care of it?

[MarkHaoxiang]

@matteobettini Have you started implementing the common multiagent nn module? If not I'll take a shot at it this week.

[matteobettini]

Nope, yo can take a shot!

So what we want is to do the exact same things but using vmap like #1497 (see that pr for context)

[matteobettini]

@MarkHaoxiang we are posticipating that refactoring a bit to wait for some features, so we can go ahead with this as is.
@vmoens could you have a look?

[vmoens]

Great work! A couple of comments regarding formatting

[vmoens]

Tests are not passing, presumably because we moved the scripts from .circleci to .github for the CI

[MarkHaoxiang]

Thanks for the check - I've adjusted the formatting as requested

[vmoens]

Almost there!

[matteobettini]

CNN tests are failing

[MarkHaoxiang]

Fixed a bug with reshaping the tensor step in the centralised/parameter sharing case.

output = (
                    output.view(*output.shape[:-1], self.n_agent_outputs)
                    .unsqueeze(-2)
                    .expand(*output.shape[:-1], self.n_agents, self.n_agent_outputs)
                )

@matteobettini whats the motivation behind expanding the tensor like this (for the MLP version) compared to

output = output.unsqueeze(-2)
output = output.expand(*output.shape[:-2], self.n_agents, n_agent_outputs)

[matteobettini]

They seem equivalent, but in the first case the view acts as an implicit assert on the shape, so it is more readable in my opinion.

[matteobettini]

What was the problem with the previous way? Why was it a bug?

EDIT:

ah i see, when you broke it into multiline the modifications of each line was applied on different tensors

i would still keep the view before to make sure

[MarkHaoxiang]

Sure, I've changed both to use the multi-line version but with the view

[vmoens]

LGTM! Thanks for this