Skip to content

Raise error for data-parallel with benchmark_throughput#16737

Merged
youkaichao merged 3 commits intovllm-project:mainfrom
kartikx:dev/kartikx/data-parallel-error
Apr 21, 2025
Merged

Raise error for data-parallel with benchmark_throughput#16737
youkaichao merged 3 commits intovllm-project:mainfrom
kartikx:dev/kartikx/data-parallel-error

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
Copy link

github-actions bot commented Apr 16, 2025

👋 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>
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>
@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?

@kartikx
Copy link
Contributor Author

kartikx commented Apr 28, 2025

@Potabk What version of vLLM are you using?

@Potabk
Copy link
Contributor

Potabk commented Apr 29, 2025

@Potabk What version of vLLM are you using?

The latest benchmark script

jikunshang pushed a commit to jikunshang/vllm that referenced this pull request Apr 29, 2025
@kartikx @simon-mo
Raise error for data-parallel with benchmark_throughput (vllm-project…
93f1056 
…#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 29, 2025

I was referring to the version of vLLM you have installed. You can check this via pip show vllm.

@Potabk
Copy link
Contributor

Potabk commented Apr 29, 2025

I was referring to the version of vLLM you have installed. You can check this via pip show vllm.

Ok, never mind it, I think it is the inappropriate version, the latest version of vllm is ok

lk-chen pushed a commit to lk-chen/vllm that referenced this pull request Apr 29, 2025
@kartikx @simon-mo @lk-chen
Raise error for data-parallel with benchmark_throughput (vllm-project…
4e689dd 
…#16737)

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

Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
Co-authored-by: Simon Mo <simon.mo@hey.com>
Signed-off-by: Agata Dobrzyniewicz <adobrzyniewicz@habana.ai>
RichardoMrMu pushed a commit to RichardoMrMu/vllm that referenced this pull request May 12, 2025
@kartikx @simon-mo
Raise error for data-parallel with benchmark_throughput (vllm-project…
0df6a15 
…#16737)

Signed-off-by: Kartik Ramesh <kartikx2000@gmail.com>
Co-authored-by: Simon Mo <simon.mo@hey.com>
Signed-off-by: Mu Huai <tianbowen.tbw@antgroup.com>
@ckhordiasma ckhordiasma mentioned this pull request May 14, 2025
Merged
@zhewenl zhewenl mentioned this pull request Aug 16, 2025
Merged
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