Skip to content

[1/N][trainer] feat: bagel flowgrpo integration #132

Merged
zhtmike merged 5 commits into
verl-project:mainfrom
zhtmike:interface
Jun 3, 2026
Merged

[1/N][trainer] feat: bagel flowgrpo integration #132
zhtmike merged 5 commits into
verl-project:mainfrom
zhtmike:interface

Conversation

@zhtmike
Copy link
Copy Markdown
Collaborator

@zhtmike zhtmike commented Jun 2, 2026

What does this PR do?

one piece of the PR #66

  • add interface for integrating non-diffusers model to diffusers FSDP engine.
  • add a option for non-diffusers model prefix (e.g., layer. instead of transformer_blocks)
  • add a guard for empty lora returns
  • add a guard for to and enable_gradient_checkpointing implementation for non-diffusers model
  • add attention mask returns in agent loop (required by Bagel, etc)

Add concise overview of what this PR aims to achieve or accomplish. Reference related GitHub issues and PRs that help with the review.

Checklist Before Starting

  • Search for similar PRs. Paste at least one query link here: ...
  • Format the PR title as [{modules}] {type}: {description} (This will be checked by the CI)
    • {modules} include fsdp, vllm_omni, rollout, trainer, ci, training_utils, recipe, ray, worker, single_controller, misc, perf, model, algo, env, tool, ckpt, doc, data, cfg, reward, diffusion, omni, tests, docker
    • If this PR involves multiple modules, separate them with , like [diffusion, doc]
    • {type} is in feat, fix, refactor, chore, test
    • If this PR breaks any API (CLI arguments, config, function signature, etc.), add [BREAKING] to the beginning of the title.
    • Example: [BREAKING][diffusion, fsdp] feat: new rollout scheduler

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, evaluation results, etc.

API and Usage Example

Demonstrate how the API changes if any, and provide usage example(s) if possible.

# Add code snippet or script demonstrating how to use this

Design & Code Changes

Demonstrate the high-level design if this PR is complex, and list the specific changes.

Checklist Before Submitting

Important

Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.

  • Read the Contribute Guide.
  • Apply pre-commit checks: pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always
  • Add / Update the documentation.
  • Add unit or end-to-end test(s) to the CI workflow to cover all the code. If not feasible, explain why: ...

@zhtmike zhtmike requested a review from SamitHuang as a code owner June 2, 2026 08:17
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for custom model loaders in the diffusion pipeline by adding a build_module method to DiffusionModelBase and implementing _build_module_from_registry in the FSDP engine. It also adds configurable FSDP layer prefixes (fsdp_layer_prefixes) to allow flexible LoRA parameter collection, and improves robustness in prepare_model_inputs by safely handling missing prompt embeddings. Feedback on the changes suggests replacing a try-except AttributeError block with explicit hasattr and callable checks when enabling gradient checkpointing on custom modules to avoid masking internal AttributeErrors.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread verl_omni/workers/engine/fsdp/diffusers_impl.py
@zhtmike zhtmike changed the title [1/N][trainer] feat: bagel integration [1/N][trainer] feat: bagel flowgrpo integration Jun 2, 2026
@zhtmike zhtmike requested a review from AndyZhou952 June 2, 2026 08:40
@zhtmike zhtmike added the ready-for-ci read for running CI label Jun 2, 2026
@github-actions github-actions Bot removed the ready-for-ci read for running CI label Jun 2, 2026
@zhtmike zhtmike added the ready-for-ci read for running CI label Jun 2, 2026
@github-actions github-actions Bot removed the ready-for-ci read for running CI label Jun 2, 2026
@zhtmike zhtmike added the ready-for-ci read for running CI label Jun 2, 2026
@zhtmike
Copy link
Copy Markdown
Collaborator Author

zhtmike commented Jun 2, 2026

@SamitHuang PTAL

it is the nececessary interface for non-diffusers integration.
The lora weight transfer's bug should be guarded now

@github-actions github-actions Bot removed the ready-for-ci read for running CI label Jun 3, 2026
@zhtmike zhtmike added the ready-for-ci read for running CI label Jun 3, 2026
@zhtmike zhtmike merged commit fd9cfde into verl-project:main Jun 3, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci read for running CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants