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.

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

Merged
merged 3 commits into from
Apr 21, 2025

Conversation

kartikx
Copy link
Contributor

@kartikx kartikx commented Apr 16, 2025

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!

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.

🚀

@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
Co-authored-by: Simon Mo <[email protected]>
Signed-off-by: Kartik Ramesh <[email protected]>
Signed-off-by: Kartik Ramesh <[email protected]>
@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
frieda-huang pushed a commit to frieda-huang/vllm that referenced this pull request Apr 23, 2025
…#16737)

Signed-off-by: Kartik Ramesh <[email protected]>
Co-authored-by: Simon Mo <[email protected]>
Signed-off-by: Frieda (Jingying) Huang <[email protected]>
@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
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
adobrzyn pushed a commit to HabanaAI/vllm-fork that referenced this pull request Apr 30, 2025
…#16737)

Signed-off-by: Kartik Ramesh <[email protected]>
Co-authored-by: Simon Mo <[email protected]>
Signed-off-by: Agata Dobrzyniewicz <[email protected]>
RichardoMrMu pushed a commit to RichardoMrMu/vllm that referenced this pull request May 12, 2025
minpeter pushed a commit to minpeter/vllm that referenced this pull request Jun 24, 2025
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.

4 participants