-
-
Notifications
You must be signed in to change notification settings - Fork 11.8k
[UX] Replace VLLM_ALL2ALL_BACKEND with --all2all-backend #26732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: mgoin <[email protected]>
|
Documentation preview: https://vllm--26732.org.readthedocs.build/en/26732/ |
Signed-off-by: mgoin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
There was a problem hiding this 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 a new command-line argument --all2all-backend to replace the VLLM_ALL2ALL_BACKEND environment variable, which is now deprecated. The changes span across documentation, configuration files, and various model execution layers to adopt the new configuration method. This is a good UX improvement. My review includes one high-severity comment regarding a potential logic mismatch between the implementation and the stated behavior in the pull request description concerning the precedence of the new argument versus the old environment variable.
Signed-off-by: mgoin <[email protected]>
yewentao256
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the work!
…t#26732) Signed-off-by: mgoin <[email protected]> Signed-off-by: 1994 <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]> Signed-off-by: Dhruvil Bhatt <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]> Signed-off-by: bbartels <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]> Signed-off-by: 0xrushi <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]> Signed-off-by: 0xrushi <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]>
…t#26732) Signed-off-by: mgoin <[email protected]>
Purpose
We have solidified the use of various all-to-all backends for the moe communication during expert parallelism. It is past time to graduate the environment variable to a proper config.
Old way (still works but deprecated):
VLLM_ALL2ALL_BACKEND=deepep_high_throughput vllm serve openai/gpt-oss-20b --data-parallel-size 2 --enable-expert-parallelNew way:
vllm serve openai/gpt-oss-20b --data-parallel-size 2 --enable-expert-parallel --all2all-backend deepep_high_throughputThe env var will still work by itself, but it will be overriden by the argument if both are set. Users will see a deprecation warning encouraging them to use the argument instead.
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.