Skip to content

fix: add loss_agg_mode to critics#1340

Merged
vermouth1992 merged 6 commits intoverl-project:mainfrom
tongyx361:tyx/fix/unify-loss-agg-mode
May 23, 2025
Merged

fix: add loss_agg_mode to critics#1340
vermouth1992 merged 6 commits intoverl-project:mainfrom
tongyx361:tyx/fix/unify-loss-agg-mode

Conversation

@tongyx361
Copy link
Copy Markdown
Collaborator

@tongyx361 tongyx361 commented Apr 30, 2025

What does this PR do?

This PR adds loss_agg_mode to critics.

Before submitting

  • Did you read the Contribute Guide and finish the code format check?
  • Did you make sure to update the documentations with your changes in the docs especially for breaking config etc?
  • Did you write any test cases if neccessary? Please add CI tests to your new feature.

Additional Info

  • Issue Number: none
  • Training: both
  • Inference: none

ETOgaosion
ETOgaosion previously approved these changes May 1, 2025


def compute_entropy_loss(logits, response_mask):
def compute_entropy_loss(entropy: torch.Tensor, response_mask: torch.Tensor, loss_agg_mode: str):
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.

This function changes the semantics of compute_entropy_loss, which may break others code if they use it in their own codebase,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your suggestion! I will fix it!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Taking backward compatibility for consideration, I decide to keep the default values and remove the functions for token-wise losses like entropy_loss and kl_loss. For now, this PR only adds loss_agg_mode to critics.

@tongyx361 tongyx361 changed the title fix: unify the usage of loss_agg_mode fix: add loss_agg_model to critics May 1, 2025
@tongyx361 tongyx361 requested a review from vermouth1992 May 1, 2025 15:10
@tongyx361 tongyx361 changed the title fix: add loss_agg_model to critics fix: add loss_agg_mode to critics May 1, 2025
@vermouth1992 vermouth1992 merged commit 9ddc725 into verl-project:main May 23, 2025
22 checks passed
ETOgaosion pushed a commit to Jianbing-D/verl that referenced this pull request Jun 8, 2025
# What does this PR do?

This PR adds `loss_agg_mode` to critics.

# Before submitting

- [x] Did you read the [Contribute
Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide)
and finish the [code format
check](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting)?
- [x] Did you make sure to update the documentations with your changes
in the [docs](https://github.com/volcengine/verl/tree/main/docs)
especially for breaking config etc?
- [x] Did you write any test cases if neccessary? Please add CI tests to
your new feature.

# Additional Info

- **Issue Number**: none
- **Training**: both
- **Inference**: none
wwwjn pushed a commit to wwwjn/verl that referenced this pull request Jun 10, 2025
# What does this PR do?

This PR adds `loss_agg_mode` to critics.

# Before submitting

- [x] Did you read the [Contribute
Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide)
and finish the [code format
check](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting)?
- [x] Did you make sure to update the documentations with your changes
in the [docs](https://github.com/volcengine/verl/tree/main/docs)
especially for breaking config etc?
- [x] Did you write any test cases if neccessary? Please add CI tests to
your new feature.

# Additional Info

- **Issue Number**: none
- **Training**: both
- **Inference**: none
chenjiaoAngel added a commit to chenjiaoAngel/verl that referenced this pull request Nov 14, 2025
# What does this PR do?

This PR adds `loss_agg_mode` to critics.

# Before submitting

- [x] Did you read the [Contribute
Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide)
and finish the [code format
check](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting)?
- [x] Did you make sure to update the documentations with your changes
in the [docs](https://github.com/volcengine/verl/tree/main/docs)
especially for breaking config etc?
- [x] Did you write any test cases if neccessary? Please add CI tests to
your new feature.

# Additional Info

- **Issue Number**: none
- **Training**: both
- **Inference**: none
TimurTaepov pushed a commit to giorgossideris/verl that referenced this pull request Dec 20, 2025
# What does this PR do?

This PR adds `loss_agg_mode` to critics.

# Before submitting

- [x] Did you read the [Contribute
Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide)
and finish the [code format
check](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting)?
- [x] Did you make sure to update the documentations with your changes
in the [docs](https://github.com/volcengine/verl/tree/main/docs)
especially for breaking config etc?
- [x] Did you write any test cases if neccessary? Please add CI tests to
your new feature.

# Additional Info

- **Issue Number**: none
- **Training**: both
- **Inference**: none
vyomakesh0728 added a commit to vyomakesh0728/verl that referenced this pull request Jan 22, 2026
# What does this PR do?

This PR adds `loss_agg_mode` to critics.

# Before submitting

- [x] Did you read the [Contribute
Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide)
and finish the [code format
check](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting)?
- [x] Did you make sure to update the documentations with your changes
in the [docs](https://github.com/volcengine/verl/tree/main/docs)
especially for breaking config etc?
- [x] Did you write any test cases if neccessary? Please add CI tests to
your new feature.

# Additional Info

- **Issue Number**: none
- **Training**: both
- **Inference**: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants