Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion verl/workers/rollout/vllm_rollout/vllm_omni_async_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,20 @@ async def generate(
extra_args[k] = v
sampling_kwargs["extra_args"] = extra_args
if lora_request is not None:
sampling_kwargs["lora_request"] = lora_request
# vllm-omni >=0.19 renames lora_request/lora_scale to the plural
# lora_requests/lora_scales (list form) for multi-LoRA composition.
# Detect which field the installed version exposes so this adapter
# works on both pre- and post-rename releases.
if "lora_requests" in OmniDiffusionSamplingParams.__dataclass_fields__:
sampling_kwargs["lora_requests"] = [lora_request]
# On the post-rename API the singular "lora_scale" key from
# sampling_params would have fallen into extra_args above (no
# such field on the dataclass); lift it back out into the
# plural lora_scales so the scale actually takes effect.
if "lora_scale" in extra_args:
sampling_kwargs["lora_scales"] = [extra_args.pop("lora_scale")]
else:
sampling_kwargs["lora_request"] = lora_request
Comment on lines +178 to +187
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.

high

The PR description and the added code comments (lines 174-175) explicitly mention that both lora_request and lora_scale are being renamed to their plural forms (lora_requests and lora_scales) in vllm-omni >= 0.19. However, the current implementation only handles the forward-compatibility for lora_request.

If a user passes lora_scale in sampling_params, it will be moved to extra_args by the loop at lines 167-171 when running on a newer version of the library. This value will then be ignored by the OmniDiffusionSamplingParams constructor. To ensure full forward-compatibility as intended, lora_scale should also be mapped to lora_scales (as a single-element list) when the plural API is detected.

Suggested change
if "lora_requests" in OmniDiffusionSamplingParams.__dataclass_fields__:
sampling_kwargs["lora_requests"] = [lora_request]
else:
sampling_kwargs["lora_request"] = lora_request
if "lora_requests" in OmniDiffusionSamplingParams.__dataclass_fields__:
sampling_kwargs["lora_requests"] = [lora_request]
if "lora_scale" in extra_args:
sampling_kwargs["lora_scales"] = [extra_args.pop("lora_scale")]
else:
sampling_kwargs["lora_request"] = lora_request

diffusion_sampling_params = OmniDiffusionSamplingParams(**sampling_kwargs)

# Call AsyncOmni.generate() with the correct API
Expand Down