[Chore]:Extract math and argparse utilities to separate modules#27188
[Chore]:Extract math and argparse utilities to separate modules#27188vllm-bot merged 14 commits intovllm-project:mainfrom
Conversation
There was a problem hiding this comment.
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.
vllm/utils/__init__.py
Outdated
|
|
||
| def round_down(x: int, y: int) -> int: | ||
| return (x // y) * y | ||
| # Math utilities - imported from math_utils module |
There was a problem hiding this comment.
Let's update vLLM code to use the new module-specific imports
|
Documentation preview: https://vllm--27188.org.readthedocs.build/en/27188/ |
Fixed syntax error where an incomplete 'from vllm.utils import ('
statement was left in the file, causing compilation failures.
Signed-off-by: Yeshwanth Surya <yeshsurya@gmail.com>
- 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 <yeshsurya@gmail.com>
5a024f8 to
9b5a5fc
Compare
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 <yeshsurya@gmail.com>
ef0d520 to
92535f8
Compare
|
/gemini review |
There was a problem hiding this comment.
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.
…-project#27188) Signed-off-by: Yeshwanth Surya <yeshsurya@gmail.com> Signed-off-by: Yeshwanth N <yeshsurya@gmail.com> Signed-off-by: yeshsurya <yeshsurya@gmail.com>
…-project#27188) Signed-off-by: Yeshwanth Surya <yeshsurya@gmail.com> Signed-off-by: Yeshwanth N <yeshsurya@gmail.com> Signed-off-by: yeshsurya <yeshsurya@gmail.com>
…-project#27188) Signed-off-by: Yeshwanth Surya <yeshsurya@gmail.com> Signed-off-by: Yeshwanth N <yeshsurya@gmail.com> Signed-off-by: yeshsurya <yeshsurya@gmail.com>
### What this PR does / why we need it? adapt vllm-ascend main branch with vllm releases/v0.11.1 fix `forward context not set` in test_vlm.py caused by: vllm-project/vllm#23207 fix import `cdiv round` failed caused by: vllm-project/vllm#27188 fix import `init_cached_hf_modules` failed caused by: vllm-project/vllm#27567 adapt triton kernel `fused_recurrent_gated_delta_rule_fwd_kernel` caused by: vllm-project/vllm#27654 - remove unused code in sigmoid_gating.py - `class FusedRecurrentFunction` , `fused_recurrent_gated_delta_rule`, `fused_recurrent_gated_delta_rule_fwd` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? CI - vLLM version: v0.11.0 - vLLM main: vllm-project/vllm@83f478b Signed-off-by: 22dimensions <waitingwind@foxmail.com>
### What this PR does / why we need it? adapt vllm-ascend main branch with vllm releases/v0.11.1 fix `forward context not set` in test_vlm.py caused by: vllm-project/vllm#23207 fix import `cdiv round` failed caused by: vllm-project/vllm#27188 fix import `init_cached_hf_modules` failed caused by: vllm-project/vllm#27567 adapt triton kernel `fused_recurrent_gated_delta_rule_fwd_kernel` caused by: vllm-project/vllm#27654 - remove unused code in sigmoid_gating.py - `class FusedRecurrentFunction` , `fused_recurrent_gated_delta_rule`, `fused_recurrent_gated_delta_rule_fwd` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? CI - vLLM version: v0.11.0 - vLLM main: vllm-project/vllm@83f478b Signed-off-by: 22dimensions <waitingwind@foxmail.com> Signed-off-by: luolun <luolun1995@cmbchina.com>
### What this PR does / why we need it? adapt vllm-ascend main branch with vllm releases/v0.11.1 fix `forward context not set` in test_vlm.py caused by: vllm-project/vllm#23207 fix import `cdiv round` failed caused by: vllm-project/vllm#27188 fix import `init_cached_hf_modules` failed caused by: vllm-project/vllm#27567 adapt triton kernel `fused_recurrent_gated_delta_rule_fwd_kernel` caused by: vllm-project/vllm#27654 - remove unused code in sigmoid_gating.py - `class FusedRecurrentFunction` , `fused_recurrent_gated_delta_rule`, `fused_recurrent_gated_delta_rule_fwd` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? CI - vLLM version: v0.11.0 - vLLM main: vllm-project/vllm@83f478b Signed-off-by: 22dimensions <waitingwind@foxmail.com> Signed-off-by: hwhaokun <haokun0405@163.com>
### What this PR does / why we need it? adapt vllm-ascend main branch with vllm releases/v0.11.1 fix `forward context not set` in test_vlm.py caused by: vllm-project/vllm#23207 fix import `cdiv round` failed caused by: vllm-project/vllm#27188 fix import `init_cached_hf_modules` failed caused by: vllm-project/vllm#27567 adapt triton kernel `fused_recurrent_gated_delta_rule_fwd_kernel` caused by: vllm-project/vllm#27654 - remove unused code in sigmoid_gating.py - `class FusedRecurrentFunction` , `fused_recurrent_gated_delta_rule`, `fused_recurrent_gated_delta_rule_fwd` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? CI - vLLM version: v0.11.0 - vLLM main: vllm-project/vllm@83f478b Signed-off-by: 22dimensions <waitingwind@foxmail.com> Signed-off-by: nsdie <yeyifan@huawei.com>
…-project#27188) Signed-off-by: Yeshwanth Surya <yeshsurya@gmail.com> Signed-off-by: Yeshwanth N <yeshsurya@gmail.com> Signed-off-by: yeshsurya <yeshsurya@gmail.com>
### What this PR does / why we need it? adapt vllm-ascend main branch with vllm releases/v0.11.1 fix `forward context not set` in test_vlm.py caused by: vllm-project/vllm#23207 fix import `cdiv round` failed caused by: vllm-project/vllm#27188 fix import `init_cached_hf_modules` failed caused by: vllm-project/vllm#27567 adapt triton kernel `fused_recurrent_gated_delta_rule_fwd_kernel` caused by: vllm-project/vllm#27654 - remove unused code in sigmoid_gating.py - `class FusedRecurrentFunction` , `fused_recurrent_gated_delta_rule`, `fused_recurrent_gated_delta_rule_fwd` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? CI - vLLM version: v0.11.0 - vLLM main: vllm-project/vllm@83f478b Signed-off-by: 22dimensions <waitingwind@foxmail.com>
Purpose
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
supported_models.mdandexamplesfor a new model.