Skip to content

[Chore]:Extract math and argparse utilities to separate modules #27188

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 14 commits into from
Oct 26, 2025
Merged

[Chore]:Extract math and argparse utilities to separate modules #27188

merged 14 commits into from
Oct 26, 2025

Conversation

@yeshsurya
Copy link
Contributor

@yeshsurya yeshsurya commented Oct 20, 2025 •
edited by github-actions bot
Loading

Purpose

  • Extract math utilities (cdiv, round_up, round_down, etc.) to math_utils.py
  • Extract argument parsing classes (FlexibleArgumentParser, etc.) to argparse_utils.py
  • Reduce init.py from 1,293 to 819 lines (37% reduction)
  • Maintain full backward compatibility via re-exports

Contributes to #26900

Test Plan

Verify that the refactoring of math and argparse utilities into separate modules (vllm/utils/math_utils.py and vllm/utils/argparse_utils.py)
maintains backward compatibility and does not break existing functionality

Test Result

tests/utils_/test_utils.py
Passed

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

gemini-code-assist[bot]
gemini-code-assist bot reviewed Oct 20, 2025
View reviewed changes
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 refactors math and argparse utilities into separate modules, which is a great step for improving code organization and maintainability. The changes correctly maintain backward compatibility by re-exporting the moved utilities. My review focuses on the newly created files, where I've identified a few issues: a missing type hint that could lead to a runtime error, a leftover debug print statement, and a couple of instances of incorrect string formatting for error and log messages. These issues could impact usability and debugging. I've provided specific suggestions to address them.

DarkLight1337
DarkLight1337 reviewed Oct 20, 2025
View reviewed changes
vllm/utils/__init__.py Outdated

def round_down(x: int, y: int) -> int:
return (x // y) * y
# Math utilities - imported from math_utils module
Copy link
Member

@DarkLight1337 DarkLight1337 Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update vLLM code to use the new module-specific imports

@mergify
Copy link

mergify bot commented Oct 21, 2025

Documentation preview: https://vllm--27188.org.readthedocs.build/en/27188/

@mergify mergify bot added documentation Improvements or additions to documentation frontend performance Performance-related issues qwen Related to Qwen models gpt-oss Related to GPT-OSS models labels Oct 21, 2025
@yeshsurya yeshsurya force-pushed the feature/extract-argparse-utils branch from 81a9253 to 7b71513 Compare October 25, 2025 11:31
@mergify mergify bot removed the needs-rebase label Oct 25, 2025
@yeshsurya yeshsurya force-pushed the feature/extract-argparse-utils branch from 21c991b to 75c60a0 Compare October 25, 2025 11:49
@yeshsurya
[Fix]: Remove incomplete import statement in models/utils.py
0252ee8 
Fixed syntax error where an incomplete 'from vllm.utils import ('
statement was left in the file, causing compilation failures.

Signed-off-by: Yeshwanth Surya <[email protected]>
@yeshsurya
[Fix]: Resolve ruff linting errors
9b5a5fc 
- Remove unused imports from vllm/utils/__init__.py (argparse, collections, regex)
- Add missing 'cache' import from functools
- Fix duplicate imports in multiple files causing F811 redefinition errors:
  - tests/kernels/attention/test_deepgemm_attention.py
  - vllm/model_executor/layers/fused_moe/config.py
  - vllm/model_executor/layers/fused_moe/layer.py
  - vllm/v1/attention/backends/flashinfer.py
  - vllm/v1/worker/tpu_model_runner.py

All imports now use the specific module paths (e.g., vllm.utils.math_utils)
instead of importing from vllm.utils directly.

Signed-off-by: Yeshwanth Surya <[email protected]>
@yeshsurya yeshsurya force-pushed the feature/extract-argparse-utils branch from 5a024f8 to 9b5a5fc Compare October 25, 2025 12:00
@yeshsurya
[Fix]: Resolve import ordering issues flagged by ruff
92535f8 
Fixed import ordering in three files to comply with ruff's isort rules:
- vllm/entrypoints/utils.py: Reordered transformers_utils before utils imports
- vllm/utils/__init__.py: Moved platform_utils after specialized module imports
- vllm/utils/deep_gemm.py: Reordered import_utils before math_utils

These changes resolve the ruff-check failures in the pre-commit hooks.

Signed-off-by: yeshsurya <[email protected]>
@yeshsurya yeshsurya force-pushed the feature/extract-argparse-utils branch from ef0d520 to 92535f8 Compare October 25, 2025 12:32
@DarkLight1337
Copy link
Member

DarkLight1337 commented Oct 26, 2025

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Oct 26, 2025
View reviewed changes
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 refactors the codebase by extracting math and argparse utilities into separate modules, vllm/utils/math_utils.py and vllm/utils/argparse_utils.py respectively. This is a good improvement for code organization and maintainability. The changes are well-executed, with imports updated across the codebase and backward compatibility maintained through re-exports in vllm/utils/__init__.py. I have reviewed the changes and found no issues. The refactoring is clean and a positive step for the project's structure.

DarkLight1337
DarkLight1337 approved these changes Oct 26, 2025
View reviewed changes
Copy link
Member

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@github-project-automation github-project-automation bot moved this from To Triage to Ready in gpt-oss Issues & Enhancements Oct 26, 2025
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) October 26, 2025 07:21
@vllm-bot vllm-bot merged commit 71b1c8b into vllm-project:main Oct 26, 2025
19 of 22 checks passed
xuhaolei pushed a commit to ZJU-REAL/EasySteer-vllm-v1 that referenced this pull request Oct 27, 2025
@yeshsurya @xuhaolei
[Chore]:Extract math and argparse utilities to separate modules (vllm…
7230f51 
…-project#27188)

Signed-off-by: Yeshwanth Surya <[email protected]>
Signed-off-by: Yeshwanth N <[email protected]>
Signed-off-by: yeshsurya <[email protected]>
@RobMulla RobMulla mentioned this pull request Oct 27, 2025
Closed
1 task
@maobaolong maobaolong mentioned this pull request Oct 28, 2025
Merged
ilmarkov pushed a commit to neuralmagic/vllm that referenced this pull request Nov 7, 2025
@yeshsurya @ilmarkov
[Chore]:Extract math and argparse utilities to separate modules (vllm…
40ac336 
…-project#27188)

Signed-off-by: Yeshwanth Surya <[email protected]>
Signed-off-by: Yeshwanth N <[email protected]>
Signed-off-by: yeshsurya <[email protected]>
ZhengHongming888 pushed a commit to ZhengHongming888/vllm that referenced this pull request Nov 8, 2025
@yeshsurya @ZhengHongming888
[Chore]:Extract math and argparse utilities to separate modules (vllm…
32663f4 
…-project#27188)

Signed-off-by: Yeshwanth Surya <[email protected]>
Signed-off-by: Yeshwanth N <[email protected]>
Signed-off-by: yeshsurya <[email protected]>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
@yeshsurya @rtourgeman
[Chore]:Extract math and argparse utilities to separate modules (vllm…
a31e07e 
…-project#27188)

Signed-off-by: Yeshwanth Surya <[email protected]>
Signed-off-by: Yeshwanth N <[email protected]>
Signed-off-by: yeshsurya <[email protected]>
@22dimensions 22dimensions mentioned this pull request Nov 11, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DarkLight1337 DarkLight1337 DarkLight1337 approved these changes

@noooop noooop Awaiting requested review from noooop noooop is a code owner

@NickLucche NickLucche Awaiting requested review from NickLucche NickLucche is a code owner

@gshtras gshtras Awaiting requested review from gshtras

@mgoin mgoin Awaiting requested review from mgoin mgoin is a code owner

@tlrmchlsmth tlrmchlsmth Awaiting requested review from tlrmchlsmth tlrmchlsmth is a code owner

@WoosukKwon WoosukKwon Awaiting requested review from WoosukKwon WoosukKwon is a code owner

@yewentao256 yewentao256 Awaiting requested review from yewentao256 yewentao256 is a code owner

@robertgshaw2-redhat robertgshaw2-redhat Awaiting requested review from robertgshaw2-redhat robertgshaw2-redhat is a code owner

@simon-mo simon-mo Awaiting requested review from simon-mo simon-mo is a code owner

@aarnphm aarnphm Awaiting requested review from aarnphm aarnphm is a code owner

@heheda12345 heheda12345 Awaiting requested review from heheda12345 heheda12345 is a code owner

@njhill njhill Awaiting requested review from njhill njhill is a code owner

@ywang96 ywang96 Awaiting requested review from ywang96 ywang96 is a code owner

@comaniac comaniac Awaiting requested review from comaniac comaniac is a code owner

@alexm-redhat alexm-redhat Awaiting requested review from alexm-redhat alexm-redhat is a code owner

@ApostaC ApostaC Awaiting requested review from ApostaC ApostaC is a code owner

@LucasWilkinson LucasWilkinson Awaiting requested review from LucasWilkinson LucasWilkinson is a code owner

@zou3519 zou3519 Awaiting requested review from zou3519 zou3519 is a code owner

@youkaichao youkaichao Awaiting requested review from youkaichao youkaichao is a code owner

@ProExpertProg ProExpertProg Awaiting requested review from ProExpertProg ProExpertProg is a code owner

@chaunceyjiang chaunceyjiang Awaiting requested review from chaunceyjiang chaunceyjiang is a code owner

@22quinn 22quinn Awaiting requested review from 22quinn 22quinn is a code owner

@pavanimajety pavanimajety Awaiting requested review from pavanimajety pavanimajety is a code owner

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@sangstar sangstar

Labels

documentation Improvements or additions to documentation frontend gpt-oss Related to GPT-OSS models performance Performance-related issues qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm speculative-decoding structured-output tpu Related to Google TPUs v1

Projects

Structured Output
Status: Done
gpt-oss Issues & Enhancements
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yeshsurya @DarkLight1337 @sangstar @vllm-bot