[ppo] feat: add critic valuehead model support for multi-modal PPO by Yangruipis · Pull Request #1839 · verl-project/verl

Yangruipis · 2025-06-04T06:54:48Z

Checklist Before Starting

Search for similar PR(s).

What does this PR do?

支持多模的 PPO，主要是复用 trl 的 AutoModelForCausalLMWithValueHead 作为 critic valuehead model

High-Level Design

Demonstrate the high-level design if this PR is complex.

Specific Changes

List the specific changes.

API

Demonstrate how the API changes if any.

Usage Example

Provide usage example(s) for easier usage.

# Add code snippet or script demonstrating how to use this

Test

For changes that can not be tested by CI (e.g., algorithm implementation, new model support), validate by experiment(s) and show results like training curve plots, evaluatuion results, etc.

Additional Info.

Issue Number: Fixes issue # or discussion # if any.
Training: [Note which backend this PR will affect: FSDP, Megatron, both, or none]
Inference: [Note which backend this PR will affect: vLLM, SGLang, both, or none]

Checklist Before Submitting

Read the Contribute Guide.
Apply pre-commit checks.
Add [BREAKING] to the PR title if it breaks any API.
Update the documentation about your changes in the docs.
New CI unit test(s) are added to cover the code path.
Rely on existing unit tests on CI that covers the code path.

CLAassistant · 2025-06-04T06:55:00Z

All committers have signed the CLA.

hiyouga

Thanks for your contribution! Please see the above comments.

Btw, can we add trl to extra requirements? https://github.com/volcengine/verl/blob/main/setup.py#L49

verl/trainer/main_ppo.py

verl/utils/import_utils.py

hiyouga

LGTM

hiyouga · 2025-06-05T10:36:02Z

@Yangruipis Could you please add a unittest for this feature? https://github.com/volcengine/verl/blob/2fe47f71ab16eec06a5fcc97f10af40e2ad36752/.github/workflows/e2e_ppo_trainer.yml#L289

Yangruipis · 2025-06-05T11:20:34Z

@Yangruipis Could you please add a unittest for this feature?

https://github.com/volcengine/verl/blob/2fe47f71ab16eec06a5fcc97f10af40e2ad36752/.github/workflows/e2e_ppo_trainer.yml#L289

done~

Yangruipis · 2025-06-06T01:17:49Z

seems like trl is not installed in ci tests，i ll fix it later

Yangruipis · 2025-06-06T03:02:08Z

@hiyouga would you please help to trigger the ci workflow again? thanks

hiyouga · 2025-06-06T03:45:25Z

@Yangruipis done

vermouth1992 · 2025-06-07T00:57:27Z

AutoModelForCausalLMWithValueHead combines actor and critic into a single model. Do we simply use its value head and discard the llm head?

hiyouga · 2025-06-07T01:02:32Z

@vermouth1992 sure, because the transformers library does not provide the qwen2vl model without lm head, so we load both the lm head and the value head for multimodal critic model

Yangruipis · 2025-06-08T08:10:49Z

@hiyouga need an approvment thanks

hiyouga

LGTM! Thanks for fix

### What does this PR do? Fix bug introduced in #1839 ### Checklist Before Submitting - [x] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [x] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path.

…erl-project#1839) ### Checklist Before Starting - [ ] Search for similar PR(s). ### What does this PR do? - 支持多模的 PPO，主要是复用 trl 的 `AutoModelForCausalLMWithValueHead` 作为 critic valuehead model ### High-Level Design > Demonstrate the high-level design if this PR is complex. ### Specific Changes > List the specific changes. ### API > Demonstrate how the API changes if any. ### Usage Example > Provide usage example(s) for easier usage. ```python # Add code snippet or script demonstrating how to use this ``` ### Test > For changes that can not be tested by CI (e.g., algorithm implementation, new model support), validate by experiment(s) and show results like training curve plots, evaluatuion results, etc. ### Additional Info. - **Issue Number**: Fixes issue # or discussion # if any. - **Training**: [Note which backend this PR will affect: FSDP, Megatron, both, or none] - **Inference**: [Note which backend this PR will affect: vLLM, SGLang, both, or none] ### Checklist Before Submitting - [ ] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [ ] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path. --------- Co-authored-by: Yaowei Zheng <hiyouga@buaa.edu.cn>

### What does this PR do? Fix bug introduced in verl-project#1839 ### Checklist Before Submitting - [x] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [x] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path.

…erl-project#1839) ### Checklist Before Starting - [ ] Search for similar PR(s). ### What does this PR do? - 支持多模的 PPO，主要是复用 trl 的 `AutoModelForCausalLMWithValueHead` 作为 critic valuehead model ### High-Level Design > Demonstrate the high-level design if this PR is complex. ### Specific Changes > List the specific changes. ### API > Demonstrate how the API changes if any. ### Usage Example > Provide usage example(s) for easier usage. ```python # Add code snippet or script demonstrating how to use this ``` ### Test > For changes that can not be tested by CI (e.g., algorithm implementation, new model support), validate by experiment(s) and show results like training curve plots, evaluatuion results, etc. ### Additional Info. - **Issue Number**: Fixes issue # or discussion # if any. - **Training**: [Note which backend this PR will affect: FSDP, Megatron, both, or none] - **Inference**: [Note which backend this PR will affect: vLLM, SGLang, both, or none] ### Checklist Before Submitting - [ ] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [ ] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path. --------- Co-authored-by: Yaowei Zheng <hiyouga@buaa.edu.cn>

### What does this PR do? Fix bug introduced in verl-project#1839 ### Checklist Before Submitting - [x] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [x] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path.

…erl-project#1839) ### Checklist Before Starting - [ ] Search for similar PR(s). ### What does this PR do? - 支持多模的 PPO，主要是复用 trl 的 `AutoModelForCausalLMWithValueHead` 作为 critic valuehead model ### High-Level Design > Demonstrate the high-level design if this PR is complex. ### Specific Changes > List the specific changes. ### API > Demonstrate how the API changes if any. ### Usage Example > Provide usage example(s) for easier usage. ```python # Add code snippet or script demonstrating how to use this ``` ### Test > For changes that can not be tested by CI (e.g., algorithm implementation, new model support), validate by experiment(s) and show results like training curve plots, evaluatuion results, etc. ### Additional Info. - **Issue Number**: Fixes issue # or discussion # if any. - **Training**: [Note which backend this PR will affect: FSDP, Megatron, both, or none] - **Inference**: [Note which backend this PR will affect: vLLM, SGLang, both, or none] ### Checklist Before Submitting - [ ] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [ ] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path. --------- Co-authored-by: Yaowei Zheng <hiyouga@buaa.edu.cn>

### What does this PR do? Fix bug introduced in verl-project#1839 ### Checklist Before Submitting - [x] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [x] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path.

…erl-project#1839) ### Checklist Before Starting - [ ] Search for similar PR(s). ### What does this PR do? - 支持多模的 PPO，主要是复用 trl 的 `AutoModelForCausalLMWithValueHead` 作为 critic valuehead model ### High-Level Design > Demonstrate the high-level design if this PR is complex. ### Specific Changes > List the specific changes. ### API > Demonstrate how the API changes if any. ### Usage Example > Provide usage example(s) for easier usage. ```python # Add code snippet or script demonstrating how to use this ``` ### Test > For changes that can not be tested by CI (e.g., algorithm implementation, new model support), validate by experiment(s) and show results like training curve plots, evaluatuion results, etc. ### Additional Info. - **Issue Number**: Fixes issue # or discussion # if any. - **Training**: [Note which backend this PR will affect: FSDP, Megatron, both, or none] - **Inference**: [Note which backend this PR will affect: vLLM, SGLang, both, or none] ### Checklist Before Submitting - [ ] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [ ] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path. --------- Co-authored-by: Yaowei Zheng <hiyouga@buaa.edu.cn>

### What does this PR do? Fix bug introduced in verl-project#1839 ### Checklist Before Submitting - [x] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [x] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path.

…erl-project#1839) ### Checklist Before Starting - [ ] Search for similar PR(s). ### What does this PR do? - 支持多模的 PPO，主要是复用 trl 的 `AutoModelForCausalLMWithValueHead` 作为 critic valuehead model ### High-Level Design > Demonstrate the high-level design if this PR is complex. ### Specific Changes > List the specific changes. ### API > Demonstrate how the API changes if any. ### Usage Example > Provide usage example(s) for easier usage. ```python # Add code snippet or script demonstrating how to use this ``` ### Test > For changes that can not be tested by CI (e.g., algorithm implementation, new model support), validate by experiment(s) and show results like training curve plots, evaluatuion results, etc. ### Additional Info. - **Issue Number**: Fixes issue # or discussion # if any. - **Training**: [Note which backend this PR will affect: FSDP, Megatron, both, or none] - **Inference**: [Note which backend this PR will affect: vLLM, SGLang, both, or none] ### Checklist Before Submitting - [ ] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [ ] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path. --------- Co-authored-by: Yaowei Zheng <hiyouga@buaa.edu.cn>

### What does this PR do? Fix bug introduced in verl-project#1839 ### Checklist Before Submitting - [x] Read the [Contribute Guide](https://github.com/volcengine/verl?tab=readme-ov-file#contribution-guide). - [x] Apply [pre-commit checks](https://github.com/volcengine/verl?tab=readme-ov-file#code-linting-and-formatting). - [ ] Add `[BREAKING]` to the PR title if it breaks any API. - [ ] Update the documentation about your changes in the [docs](https://github.com/volcengine/verl/tree/main/docs). - [ ] New CI unit test(s) are added to cover the code path. - [ ] Rely on existing unit tests on CI that covers the code path.

feat: ppo critic support for multi-modal models

04db524

Yangruipis force-pushed the feat/support_anymodel_ppo branch from 526bf00 to 04db524 Compare June 4, 2025 07:43

vermouth1992 requested a review from hiyouga June 5, 2025 05:28

hiyouga requested changes Jun 5, 2025

View reviewed changes

verl/trainer/main_ppo.py Outdated Show resolved Hide resolved

verl/utils/import_utils.py Outdated Show resolved Hide resolved

fix: add trl to requirements.txt

57849a6

hiyouga previously approved these changes Jun 5, 2025

View reviewed changes

tests: add e2e test

89b2110

Yangruipis dismissed hiyouga’s stale review via 89b2110 June 5, 2025 11:19

hiyouga previously approved these changes Jun 5, 2025

View reviewed changes

fix: tests

87e8e1b

Yangruipis dismissed hiyouga’s stale review via 87e8e1b June 6, 2025 01:46

hiyouga added 2 commits June 7, 2025 08:44

Update setup.py

70e5b03

Update e2e_ppo_trainer.yml

0ec4ca0

hiyouga approved these changes Jun 9, 2025

View reviewed changes

hiyouga merged commit 5aa1b04 into verl-project:main Jun 9, 2025
36 checks passed

hiyouga mentioned this pull request Jun 9, 2025

[worker] fix: do not break dynamic bsz in dp critic #1922

Merged

6 tasks

Conversation

Yangruipis commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist Before Starting

What does this PR do?

High-Level Design

Specific Changes

API

Usage Example

Test

Additional Info.

Checklist Before Submitting

Uh oh!

CLAassistant commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hiyouga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hiyouga left a comment

Choose a reason for hiding this comment

Uh oh!

hiyouga commented Jun 5, 2025

Uh oh!

Yangruipis commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yangruipis commented Jun 6, 2025

Uh oh!

Yangruipis commented Jun 6, 2025

Uh oh!

hiyouga commented Jun 6, 2025

Uh oh!

vermouth1992 commented Jun 7, 2025

Uh oh!

hiyouga commented Jun 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yangruipis commented Jun 8, 2025

Uh oh!

hiyouga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Yangruipis commented Jun 4, 2025 •

edited

Loading

CLAassistant commented Jun 4, 2025 •

edited

Loading

Yangruipis commented Jun 5, 2025 •

edited

Loading

hiyouga commented Jun 7, 2025 •

edited

Loading