[Algorithm] QMixer loss and multiagent models

[matteobettini]

No description provided.

[matteobettini]

@Acciorocketships it would be nice to add a QGNN mixer in modules/models/multiagent.py.
implementing the mixer interface

[Acciorocketships]

What about the QGNN model? If that's not already implemented, I would prioritise that. It has a much larger impact on performance.

[matteobettini]

Yeah that is just a normal gnn right? I am thinking to make a MultiagentGNN as i made the MLP.
So that it can be used also in MADDPG and so on.

I can take care of that but the mixer is up to you

[matteobettini]

Ready for review @vmoens

[matteobettini]

Thanks a lot! Will fix.

Most of the comments on the new loss are on parts that have been copy-pasted from DQN.
Should i modify that too when I fix those?

[vmoens]

Thanks a lot! Will fix.

Most of the comments on the new loss are on parts that have been copy-pasted from DQN. Should i modify that too when I fix those?

You mean the loss_function default? Yeah sure makes sense

[matteobettini]

Not only the default, basically all of the comments.
I am just saying that most of the code of qmixerloss comes from dqnloss.

So your review comments apply to dqnloss too.
I'll modify that too when i fix them

[vmoens]

Oh i see what you mean
Better to fix them but not mandatory. I guess my point was that it's better to fix things before merging than after, but that does not mean that you need to take care of everything in this PR.
Those changes should not be too hard to implement though. Careful with bc-breaking changes, eg the device of DQN should be discouraged but not taken away all at once

[matteobettini]

this change you requested makes it bc-breaking

[matteobettini]

do you want me to still allow it but warn?

[vmoens]

no if tests pass without needing it we're good without it

[matteobettini]

i thin they pass

[vmoens]

Tests are failing on GPU tests:

test/test_cost.py::TestQMixer::test_qmix_batcher[categorical-device0-True-2] FAILED [  4%]
___________ TestQMixer.test_qmix_batcher[categorical-device0-True-2] ___________
Traceback (most recent call last):
  File "/work/env/lib/python3.9/site-packages/tensordict/nn/common.py", line 1178, in forward
    raise err
  File "/work/env/lib/python3.9/site-packages/tensordict/nn/common.py", line 1164, in forward
    tensors = self._call_module(tensors, **kwargs)
  File "/work/env/lib/python3.9/site-packages/tensordict/nn/common.py", line 1121, in _call_module
    out = self.module(*tensors, **kwargs)
  File "/work/env/lib/python3.9/site-packages/torch/nn/modules/module.py", line 1522, in _wrapped_call_impl
    return self._call_impl(*args, **kwargs)
  File "/work/env/lib/python3.9/site-packages/torch/nn/modules/module.py", line 1531, in _call_impl
    return forward_call(*args, **kwargs)
  File "/work/env/lib/python3.9/site-packages/tensordict/nn/functional_modules.py", line 567, in new_fun
    return getattr(type(self), fun_name)(self, *args, **kwargs)
  File "/work/env/lib/python3.9/site-packages/torch/nn/modules/linear.py", line 114, in forward
    return F.linear(input, self.weight, self.bias)
RuntimeError: Expected all tensors to be on the same device, but found at least two devices, cuda:0 and cpu! (when checking argument for argument mat2 in method wrapper_CUDA_mm)

[matteobettini]

oh i am seeing now that some are failing

[vmoens]

There is still this to be fixed

test/test_cost.py::TestQMixer::test_qmix_batcher[categorical-device0-False-3] - assert not True

[matteobettini]

There is still this to be fixed

Thanks a lot and sorry for not noticing that.

For the future, can you explain to me where do you see such failures?
I was opening the gpu tests pipelines but I did not see that

[vmoens]

Great work! Woohoo!