Skip to content

Raise error for data-parallel with benchmark_throughput #16737

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 21, 2025
Merged

Raise error for data-parallel with benchmark_throughput #16737

merged 3 commits into from
Apr 21, 2025
+7 −0

Conversation

kartikx
Copy link
Contributor

@kartikx kartikx commented Apr 16, 2025 •
edited by github-actions bot
Loading

Running benchmark_throughput with data parallel results in a hang. This PR raises an error if --data-parallel-size is passed in.

Based on discussions in #16222

This is my first PR to vLLM so please let me know if I should add any tests. Thanks!

@github-actions GitHub Actions
Copy link

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

simon-mo
simon-mo approved these changes Apr 17, 2025
View reviewed changes
@kartikx kartikx force-pushed the dev/kartikx/data-parallel-error branch from 96c8576 to e9240ba Compare April 18, 2025 17:03
kartikx and others added 3 commits April 18, 2025 13:14
@kartikx
Raise error for data-parallel with benchmark_throughput
b50a97e 
Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
@kartikx @simon-mo
Update benchmarks/benchmark_throughput.py
62d54fc 
Co-authored-by: Simon Mo <simon.mo@hey.com>
Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
@kartikx
fix formatting
694e48a 
Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
@kartikx kartikx force-pushed the dev/kartikx/data-parallel-error branch from e9240ba to 694e48a Compare April 18, 2025 18:14
@kartikx
Copy link
Contributor Author

kartikx commented Apr 21, 2025

@simon-mo @youkaichao Could I please get an auto-merge on this PR? Thank you.

@youkaichao youkaichao merged commit 3b34fd5 into vllm-project:main Apr 21, 2025
21 checks passed
yangw-dev pushed a commit to yangw-dev/vllm that referenced this pull request Apr 21, 2025
@kartikx @simon-mo @yangw-dev
Raise error for data-parallel with benchmark_throughput (vllm-project…
31985b6 
…#16737)

Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
Co-authored-by: Simon Mo <simon.mo@hey.com>
Signed-off-by: Yang Wang <elainewy@meta.com>
dtransposed pushed a commit to dtransposed/vllm that referenced this pull request Apr 22, 2025
@kartikx @simon-mo
Raise error for data-parallel with benchmark_throughput (vllm-project…
ae2cdad 
…#16737)

Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
Co-authored-by: Simon Mo <simon.mo@hey.com>
Signed-off-by: dtransposed <damian@damian-ml-machine.europe-west3-b.c.jetbrains-grazie.internal>
frieda-huang pushed a commit to frieda-huang/vllm that referenced this pull request Apr 23, 2025
@kartikx @simon-mo @frieda-huang
Raise error for data-parallel with benchmark_throughput (vllm-project…
ca1e582 
…#16737)

Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
Co-authored-by: Simon Mo <simon.mo@hey.com>
Signed-off-by: Frieda (Jingying) Huang <jingyingfhuang@gmail.com>
liuzijing2014 pushed a commit to liuzijing2014/vllm that referenced this pull request Apr 25, 2025
@kartikx @simon-mo @liuzijing2014
Raise error for data-parallel with benchmark_throughput (vllm-project…
42a8596 
…#16737)

Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
Co-authored-by: Simon Mo <simon.mo@hey.com>
Signed-off-by: Zijing Liu <liuzijing2014@gmail.com>
liuzijing2014 pushed a commit to liuzijing2014/vllm that referenced this pull request Apr 25, 2025
@kartikx @simon-mo @liuzijing2014
Raise error for data-parallel with benchmark_throughput (vllm-project…
e41d044 
…#16737)

Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
Co-authored-by: Simon Mo <simon.mo@hey.com>
@Potabk
Copy link
Contributor

Potabk commented Apr 28, 2025

I encountered a problem when running this script， AttributeError: 'Namespace' object has no attribute 'data_parallel_size'. Did you mean: 'tensor_parallel_size'?, I think the command line parameter is not defined --data-parallel-size ,do we need add extra kwargs?

wuisawesome pushed a commit to character-tech/vllm that referenced this pull request Apr 28, 2025
@kartikx @simon-mo @wuisawesome
Raise error for data-parallel with benchmark_throughput (vllm-project…
2637bee 
…#16737)

Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
Co-authored-by: Simon Mo <simon.mo@hey.com>
@kartikx
Copy link
Contributor Author

kartikx commented Apr 28, 2025

@Potabk What version of vLLM are you using?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simon-mo simon-mo simon-mo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kartikx @Potabk @simon-mo @youkaichao