Skip to content

Conversation

@erictang000
Copy link
Collaborator

@erictang000 erictang000 commented Jan 15, 2026

Upgrading vllm to latest (minor changes).

Needed for #885 and should subsume #882

Upgrades transformer-engine from 2.9.0 -> 2.10.0 for the megatron backend due to incompatibility with triton 3.5 (required by torch 2.9.0)

Keeps the vllm_engine.py path backwards compatible for versions of vllm < 0.11.2 (important for flash-rl integration)

Copy link
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 upgrades vllm to version 0.13.0 and torch to 2.9.0, which is a necessary update. The changes include updating dependencies in pyproject.toml and adapting the Python code in vllm_engine.py to accommodate breaking changes in the new vllm version. The changes are generally good, but I have one suggestion to improve the readability and robustness of the API compatibility logic.

Comment on lines 368 to 375
# vllm >= 0.11.2 removed model_config from OpenAI serving APIs
is_new_api = version.parse(vllm.__version__) >= version.parse("0.11.2")
legacy_kwargs = {} if is_new_api else {"model_config": model_config}

if is_new_api:
models = OpenAIServingModels(engine, base_model_paths)
else:
models = OpenAIServingModels(engine, model_config=model_config, base_model_paths=base_model_paths)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logic to handle backward compatibility for different vllm API versions is functionally correct but could be simplified for better readability and maintainability. By grouping the version-specific logic, the code becomes easier to follow.

Additionally, the else block for OpenAIServingModels instantiation was changed to use keyword arguments. While this can improve explicitness, it's safer to retain the original positional arguments for the older API path to guarantee backward compatibility, as the function signature for that version might not support keyword arguments for these parameters.

This suggested change refactors the logic to be more grouped and uses a safer approach for the legacy path.

Suggested change
# vllm >= 0.11.2 removed model_config from OpenAI serving APIs
is_new_api = version.parse(vllm.__version__) >= version.parse("0.11.2")
legacy_kwargs = {} if is_new_api else {"model_config": model_config}
if is_new_api:
models = OpenAIServingModels(engine, base_model_paths)
else:
models = OpenAIServingModels(engine, model_config=model_config, base_model_paths=base_model_paths)
# vllm >= 0.11.2 removed model_config from OpenAI serving APIs
is_new_api = version.parse(vllm.__version__) >= version.parse("0.11.2")
legacy_kwargs = {}
if is_new_api:
models = OpenAIServingModels(engine, base_model_paths)
else:
models = OpenAIServingModels(engine, model_config, base_model_paths)
legacy_kwargs["model_config"] = model_config

@erictang000 erictang000 merged commit 20caeda into main Jan 17, 2026
4 of 5 checks passed
@erictang000 erictang000 deleted the upgrade_vllm branch January 17, 2026 00:42
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.

2 participants