Skip to content

Drop invalid multimodal rollout rows before PPO training - #588

Merged
Zhiyuan He (hzy46) merged 1 commit into
microsoft:mainfrom
ZenAlexa:contrib/drop-invalid-multimodal-training-rows
Sep 14, 2026
Merged

Zhiyuan He (hzy46) merged 1 commit into
microsoft:mainfrom
ZenAlexa:contrib/drop-invalid-multimodal-training-rows

Conversation

@ZenAlexa

@ZenAlexa Ziming Wang (ZenAlexa) commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Mark failed image rows with the existing is_drop_mask so the trainer removes them before PPO minibatch construction. This covers a missing processor, image preprocessing failures, missing image-grid metadata, and mRoPE position errors. Successful image rows and text rows retain their current behavior.

Validation

The regression exercises RolloutAdapter, PIL decoding, VERL DataProto, and the trainer's row selection. It verifies that failed image rows are removed and valid text tokens and rewards survive.

The complete tests/verl suite passes on CPU with Python 3.12.14, PyTorch 2.13.0, and VERL 0.8.0. Ruff and formatting pass for the changed files.

Signed-off-by: ZenAlexa <zimingwang945@gmail.com>
@ZenAlexa
Ziming Wang (ZenAlexa) marked this pull request as ready for review September 8, 2026 09:44
Copilot AI balanced review requested due to automatic review settings September 8, 2026 09:44

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ZenAlexa

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@ZenAlexa

Copy link
Copy Markdown
Contributor Author

Zhiyuan He (@hzy46) could you approve the Test workflow for this PR? The CLA check is now passing.

@hzy46
Zhiyuan He (hzy46) merged commit 25a0dc3 into microsoft:main Sep 14, 2026
6 checks passed
@hzy46

Copy link
Copy Markdown
Contributor

Merged. Thank you for the contribution.

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