Skip to content

[Feature] Add COMA multi-agent objective - #4151

Open
Iliamsou wants to merge 3 commits into
pytorch:mainfrom
Iliamsou:feature/marl-coma
Open

[Feature] Add COMA multi-agent objective#4151
Iliamsou wants to merge 3 commits into
pytorch:mainfrom
Iliamsou:feature/marl-coma

Conversation

@Iliamsou

Copy link
Copy Markdown

What

Adds a dedicated COMALoss objective for multi-agent reinforcement learning. The loss implements the COMA counterfactual baseline with a decentralised actor and a centralised Q-value network, including n-step Q-value targets and optional advantage normalisation.

Contents

  • torchrl/objectives/multiagent/coma.py: COMALoss implementation and helpers to construct the joint observation, joint action without the current agent, and masked joint action inputs used by the centralised critic.
  • torchrl/objectives/multiagent/__init__.py and torchrl/objectives/__init__.py: expose COMALoss.
  • test/objectives/test_coma.py: tests for the counterfactual baseline, Q-value targets, n-step returns, input construction helpers, advantage normalisation, and diagnostics.

Tests

Ran:

python -m pytest test/objectives/test_coma.py

8 passed.

@pytorch-bot

pytorch-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown

🔗 Helpful Links

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

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

❌ 4 新建 Failures, 1 Unrelated Failure

As of commit d167d23 with merge base b0eab87 (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

@meta-cla

meta-cla Bot commented Aug 24, 2026

Copy link
Copy Markdown

Hi @Iliamsou!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@github-actions github-actions Bot added Feature 新建 feature Objectives labels Aug 24, 2026
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 24, 2026
@meta-cla

meta-cla Bot commented Aug 24, 2026

Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@vmoens vmoens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a dedicated COMA objective. I found several issues that need to be addressed before this can merge:

  1. [P1] Shift the actual time dimension. In compute_value_target, next_value[:, :-1] = value_target[:, 1:] only shifts time for exactly [B, T, ...] inputs. A standard collector over an unbatched environment returns [T, n_agents, 1], in which case this shifts the agent dimension. Multiple environment batch dimensions also shift the wrong axis. Please resolve the named/final TensorDict time dimension and add a [T] regression case.

  2. [P1] Bootstrap rollout boundaries and truncations correctly. Zero-filling shifted values makes the last n_step transitions of every non-terminal fixed-length rollout use incomplete returns. Masking with done also suppresses a valid bootstrap at truncation; TorchRL return semantics distinguish this with terminated. The target critic needs to evaluate the transition next observation at boundaries rather than treating the end of the sampled batch as terminal.

  3. [P1] Honor collector validity masks. Direct .mean() and F.mse_loss(...) reductions bypass LossModule._reduce_loss, so ("collector", "mask") and shifted_valid do not exclude padded transitions. COMA commonly consumes episodic sequences, so this can train both actor and critic on padding. Please keep losses elementwise and reduce through _reduce_loss(..., tensordict=tensordict).

  4. [P2] Preserve flat NestedKey values. ("next",) + tuple(self.tensor_keys.reward) works for tuple keys but turns reward="reward" into ("next", "r", "e", "w", "a", "r", "d"). Please use the repository nested-key normalization pattern for both reward and termination keys.

The test volume is proportionate, but the current target tests only use synthetic [1, T] batches, which hides the time-axis failure. Please cover [T], a truncated boundary, automatic loss masking, and both flat and nested set_keys() values.

The public API integration also needs the repository-required documentation: a complete Sphinx-style class docstring with runnable example and paper reference, an entry in docs/source/reference/objectives_multiagent.rst, and the applicable tutorial/SOTA recipe. Since this is a new test file, please also add the executable if __name__ == "__main__": pytest.main(...) block.

@github-actions github-actions Bot added Documentation Improvements or additions to documentation CI Has to do with CI setup (e.g. wheels & builds, tests...) sota-implementations/ tutorials/ labels Aug 26, 2026
@Iliamsou
Iliamsou force-pushed the feature/marl-coma branch from a967e63 to 35879da 比较 August 26, 2026 09:14
Addresses review feedback on the COMA objective:
- Resolve the rollout time dimension dynamically instead of assuming dim 1,
  fixing compute_value_target for [T], [B, T], and multi-batch-dim inputs.
- Bootstrap on terminated rather than done, and mark rollout-boundary
  transitions with no reliable bootstrap as invalid via a shifted_valid mask
  instead of silently biasing their target low.
- Compute elementwise actor/critic/entropy losses and reduce them through
  LossModule._reduce_loss so collector masks and shifted_valid are honored
  automatically.
- Stop shredding flat NestedKey values when building the ("next", ...) reward
  and terminated keys.
- Add tests for a [T]-only rollout, a truncated boundary, automatic loss
  masking, and flat/nested set_keys() values.
- Add a full Sphinx docstring with a doctest-verified example and paper
  reference, an objectives_multiagent.rst entry, a Hydra SOTA recipe under
  sota-implementations/multiagent, and a tutorial.
Comment on lines +341 to +344
next_value = _shift_time(value_target, time_dim, fill_value=0.0)
next_valid = _shift_time(valid, time_dim, fill_value=False)
value_target = reward + self.gamma * not_terminated * next_value
valid = terminated.to(torch.bool) | next_valid

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done=True, terminated=False still shifts a value across the reset. changing the next episode's first action moved the previous target from 0.5 to 2.0. can we invalidate this bootstrap at every done boundary?

if self.normalize_advantage:
# MAPPO-style per-batch standardisation: same counterfactual
# advantage, rescaled so the actor step size is batch-invariant.
advantage = (advantage - advantage.mean()) / advantage.std().clamp_min(1e-6)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one valid actor value returns nan here. masked padding also changes the scale. can we use the effective loss mask with correction=0?

注册 for free to join this conversation on GitHub. Already have an account? 登录 to comment

标签

CI Has to do with CI setup (e.g. wheels & builds, tests...) CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Documentation Improvements or additions to documentation Feature 新建 feature Objectives sota-implementations/ tutorials/

项目

None yet

Development

Successfully merging this pull request may close these issues.

3 participants