Skip to content

refactor(diffusion): extract loss_hub for Flow-GRPO and custom loss dispatch#63

Open
niehen6174 wants to merge 7 commits into
radixark:mainfrom
niehen6174:refactor/flow-grpo-loss-hub
Open

refactor(diffusion): extract loss_hub for Flow-GRPO and custom loss dispatch#63
niehen6174 wants to merge 7 commits into
radixark:mainfrom
niehen6174:refactor/flow-grpo-loss-hub

Conversation

@niehen6174

Copy link
Copy Markdown
Collaborator

Motivation

Move Flow-GRPO PPO-clip and GRPO reward normalization out of the actor/rollout into swappable building blocks — same default behavior, ready for NFT/SFT as flag recipes.

  • Problem: ~300 lines of Flow-GRPO math lived inside actor.py; reward normalization was duplicated inline in rollout.py. There was no working hook to replace the train objective.
  • Fix: New loss_hub/ module + --loss-type flow_grpo / --custom-loss-function-path. Actor only schedules micro-batches and calls loss_fn(ctx, batch).
  • Scope: Structural refactor only. Default Flow-GRPO path is a move, not a rewrite. NFT / SFT / offline collection are follow-up PRs.

Changes

BEFORE                              AFTER
────────────────────────────────    ────────────────────────────────────
RolloutManager                      RolloutManager
  └─ GRPO norm (inline, 25 lines)     └─ grpo_normalize_rewards()  ← loss_hub
  └─ RolloutTrainDataConverter        └─ RolloutTrainDataConverter (unchanged)

FSDPTrainRayActor (~300 lines)      FSDPTrainRayActor (~20 lines of loss glue)
  └─ DiT forward                    get_diffusion_loss_function(args)
  └─ SDE logπ                       └─ flow_grpo_ppo_loss(ctx, batch)
  └─ PPO-clip
  └─ micro-batch schedule             micro-batch schedule (unchanged)
                                      backward / optim (unchanged)

Roadmap

flowchart LR
  P1["PR1: loss_hub<br/>(this)"] --> P2["PR2: NFT<br/>swap convert + loss + signals"]
  P2 --> P3["PR3: SFT<br/>swap rollout + convert + loss"]

  style P1 fill:#E8F1FF,stroke:#2F6FED
  style P2 fill:#EAF8EF,stroke:#1B7F4E
  style P3 fill:#FFF6E5,stroke:#C47D00
Loading
Follow-up Hooks to swap Notes
DiffusionNFT --custom-reward-post-process-path, --custom-convert-samples-to-train-data-path, --custom-loss-function-path Forward FM; no traj/logπ needed
SFT --rollout-function-path, convert, loss, --disable-compute-advantages-and-returns Same pattern as Miles sft_rollout + sft_loss
Collection tuning rollout params Skip unused traj/logπ once NFT lands

Extract shared diffusion loss plumbing (context, GRPO advantage helper,
flow_grpo_ppo_loss, get_diffusion_loss_function) so actor stays thin and
custom objectives can plug in via --custom-loss-function-path.
Move default per-prompt GRPO reward normalization into loss_hub so rollout
and training share one implementation; custom post-process still overrides.
…ction-path

Expose the diffusion default objective explicitly and wire the custom loss
hook that loss_hub dispatch expects, matching miles LLM customization style.
Replace inline Flow-GRPO forward/PPO logic with loss_fn dispatch and keep
upstream recompute-old-log-prob support by forwarding write flags to loss_hub.
Cover default flow_grpo_ppo selection, policy_loss alias, custom loss path
loading, and grpo_normalize_rewards mean/std behavior.
Use flow_grpo as the primary CLI name to match the Flow-GRPO recipe; keep
flow_grpo_ppo and policy_loss as aliases for backward compatibility.
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.

1 participant