Skip to content

Conversation

@paulyu12
Copy link
Collaborator

@paulyu12 paulyu12 commented Oct 27, 2025

What this PR does / why we need it?

It's a tiny bugfix in the gen_ranktable.py script. The script is an util to help setup an example case. It is used to prepare a ranktable before disaggregated prefill deployment.

Elements in local_device_ids list should be casted to int type before referred for a MOD math operation.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

No.

…to int type before referred.

Signed-off-by: paulyu12 <507435917@qq.com>
@paulyu12 paulyu12 requested a review from Potabk October 27, 2025 03:24
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 fixes a bug in the gen_ranktable.py example script where device IDs from the --local-device-ids argument were not converted to integers, causing errors in subsequent arithmetic operations. The fix is correct in its intent. My review includes a suggestion to make the implementation more robust by adding error handling for invalid inputs. This will prevent the script from crashing and provide a more user-friendly error message if a non-integer value is provided.

@github-actions
Copy link

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

@Potabk
Copy link
Collaborator

Potabk commented Oct 27, 2025

Except for Gemini's review, LGTM

…to int type before referred.

Signed-off-by: paulyu12 <507435917@qq.com>
@paulyu12
Copy link
Collaborator Author

I've adjusted according to the Gemini's review.

@wangxiyuan wangxiyuan merged commit b8796b0 into vllm-project:main Oct 27, 2025
16 checks passed
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.

3 participants