feat(reward): support HPSv2 and refine reward scheduling - #207
feat(reward): support HPSv2 and refine reward scheduling#207JingwenGu0829 wants to merge 25 commits into
Conversation
| ] | ||
| num_gpus_per_worker = 0.05 | ||
| num_cpus_per_worker = 0.05 | ||
| # Each bundle has one 0.05 slot; a second long-lived pool on the same |
There was a problem hiding this comment.
A quick discussion here: the mental model of "colocate reward" is actually "reward slots colocate with rollout&train actors". In most cases, the reward models cannot take up all the slots (e.g., in wan2.2 17gpu pickscore, the ratio of rollout engines:reward is 16:4). Maybe we should arrange these reward models in the slots available instead of giving different reward models separate pools
| } | ||
|
|
||
|
|
||
| def _sample_to_rgb_hwc_uint8(sample: Sample) -> np.ndarray: |
There was a problem hiding this comment.
This function has already been absorbed into processing_utils.py, maybe let's reuse _sample_to_rgb_hwc_uint8_frames
There was a problem hiding this comment.
I took another look at this. I think keeping the thin HPS adapter separate is cleaner here.
The shared conversion is already reused through cfhw_to_fhwc and image_or_video_to_uint8.
The remaining behavior is pretty RM-specific, including HPS rejects multi-frame outputs and uses round_normalized=True, while PickScore performs video-frame sampling and currently uses the default truncating conversion.
Reusing _sample_to_rgb_hwc_uint8_frames would therefore require another conversion flag plus a separate HPS frame-count check, while introducing an HPS-to-PickScore private-helper dependency.
It would remove very little logic, so I would prefer to keep the two small adapters separate. What do you think?
There was a problem hiding this comment.
Oh, actually images are 1-frame videos in sgl-d's output so don't worry about that~
f01c11b to
a314b7a
Compare
Minimal design for colocated reward slot allocationMirror existing rollout placement semanticsUse exactly the same effective span as rollout actor creation: span = min(num_gpus_per_engine, num_gpus_per_node)When covered = len(bundle_indices) // span * spanFor a position inside position_in_actor = position % span
is_base = position_in_actor == 0Positions after If Reward-local dispersal orderAdd no topology class. def _dispersal_order(
bundle_indices: list[int],
gpu_ids: list[int],
num_gpus_per_node: int,
num_gpus_per_engine: int,
) -> tuple[int, ...]:
span = min(num_gpus_per_engine, num_gpus_per_node)
covered = len(bundle_indices) // span * span if span > 0 else 0
def key(item: tuple[int, int, int]) -> tuple[int, int, int, int]:
position, _bundle_index, gpu_id = item
if position >= covered:
# Not occupied by a rollout actor: prefer the extra headroom.
return (0, -1, gpu_id, position)
position_in_actor = position % span
if position_in_actor == 0:
# The Ray rollout actor consumes 0.25 on this bundle; use it last.
return (1, 0, gpu_id, position)
return (0, position_in_actor, gpu_id, position)
slots = zip(range(len(bundle_indices)), bundle_indices, gpu_ids)
return tuple(bundle_index for _, bundle_index, _ in sorted(slots, key=key))The final What the stable sort doesFor normal homogeneous nodes, equal local GPU IDs appear once per node. Sorting
For two nodes × eight GPUs with four GPUs per rollout actor: For single-GPU engines all occupied bundles are bases, so the order reduces to Graceful degradationThe ordering is total for every concrete PG view:
This tradeoff is deliberate: exact balance under an information-poor unusual Shared runtime allocator
class ColocatedRewardSlots:
def __init__(self, order: tuple[int, ...]):
self._order = order
self._owners: dict[int, str] = {}
self._pool_names: set[str] = set()
def allocate(self, name: str, num_workers: int) -> list[int]:
if name in self._pool_names:
raise RuntimeError(f"{name} already owns reward slots")
start = len(self._owners)
remaining = len(self._order) - start
if num_workers > remaining:
raise RuntimeError(...actual capacity and ownership summary...)
slots = list(self._order[start : start + num_workers])
self._owners.update({slot: name for slot in slots})
self._pool_names.add(name)
return slotsThe capacity check happens before mutation, so exhaustion cannot partially Contiguous slices continue from the phase where the previous pool stopped. On For irregular placements, the proof is intentionally weaker: allocation IntegrationOnly set_manager_placement_group(
pg,
num_gpus_per_node=args.num_gpus_per_node,
num_gpus_per_engine=args.rollout_num_gpus_per_engine,
)
_pg, bundle_indices, gpu_ids = pg
_reward_slots = ColocatedRewardSlots(
_dispersal_order(
bundle_indices,
gpu_ids,
num_gpus_per_node,
num_gpus_per_engine,
)
)The lazy TestsKeep the tests reward-local and small:
No test asserts that a Miles configuration or Ray placement is invalid. |
56ac4ac to
b7f0694
Compare
| } | ||
|
|
||
|
|
||
| def _sample_to_rgb_hwc_uint8(sample: Sample) -> np.ndarray: |
There was a problem hiding this comment.
Oh, actually images are 1-frame videos in sgl-d's output so don't worry about that~
This reverts commit 41492cd.
Summary
rm_hub, with v2.0/v2.1 checkpoints and aligned prompt-image scoring.weights_only=Trueto avoid a second checkpoint copy in GPU memory during actor initialization.Design notes / known issues
Documentation and verification status. The generic HPS reward API is documented under Rewards and the runnable recipe is listed on the SD3 page. The preferred final placement/wording may need maintainer guidance. Because only short end-to-end runs have been performed and no complete reference curve or deterministic gate is committed, the recipe is marked ○ NV rather than Verified/FG.
Only one GPU reward pool may use
--colocate-reward. A colocated train actor uses 0.70 GPU and the rollout engine uses 0.25, leaving one 0.05 slot per placement-group bundle. HPS and PickScore singleton pools previously both selected the first bundle, so constructing both pools could leave the second actor pending forever.rm_hubnow claims the first colocated GPU pool and fails fast when a different pool tries to start. Mixed GPU reward types must use dedicated reward GPUs until bundle capacity is allocated across pools.model.safetensorsandmodel.fp16.safetensors, rejects the ambiguous load, and falls back to the native TransformersCLIPTextModel. The fallback completes and the end-to-end HPS run proceeds; the Miles-d team considers this a Diffusers/checkpoint-layout issue with limited performance impact. This PR does not change that upstream loader behavior.