Skip to content

Conversation

llsj14
Copy link
Contributor

@llsj14 llsj14 commented Oct 12, 2025

Purpose

I found pre-commit hook errors during the merge process. To fix these violations, I updated the import statements.

Enforce import regex as re..........................................................................Failed
- hook id: enforce-import-regex-instead-of-re
- exit code: 1

❌ vllm/platforms/cpu.py:
  Line 7: import re

💡 Found 1 violation(s).
❌ Please replace 'import re' with 'import regex as re'
   Also replace 'from re import ...' with 'from regex import ...'
✅ Allowed imports:
   - import regex as re
   - import regex

Forbid direct 'import triton'.......................................................................Failed
- hook id: forbid-direct-triton-import
- exit code: 1

❌ Forbidden direct `import triton` detected. ➤ Use `from vllm.triton_utils import triton` instead.

❌ vllm/model_executor/layers/quantization/qutlass_utils.py:17: import triton
❌ vllm/model_executor/layers/quantization/qutlass_utils.py:18: import triton.language as tl

Test Plan

Test Result


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.

Signed-off-by: Sungjae Lee <[email protected]>
Signed-off-by: Sungjae Lee <[email protected]>
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 aims to fix import violations for re and triton modules. The change to use regex instead of re is consistent with the project's conventions. However, the change to use a centralized triton import introduces a critical issue. It will cause a runtime AttributeError when Triton is not installed because the placeholder object is missing the cdiv method used in qutlass_utils.py. I've provided a comment with a suggested fix.

import triton.language as tl
from torch.library import wrap_triton

from vllm.triton_utils import triton
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

This change to use the centralized triton import utility is a good step for consistency. However, it introduces a runtime error when Triton is not installed. The TritonPlaceholder object, which is used when Triton is unavailable, does not implement the cdiv method. This file calls triton.cdiv on lines 109, 110, 127, and 128, which will lead to an AttributeError.

To resolve this, please replace all calls to triton.cdiv with the locally defined ceil_div function (line 145), which provides the same ceiling division functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe this part should be reverted?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this part should be reverted?

I think we should keep the current changes, as this could use TritonPlaceholder to avoid importing error. However, if the triton kernel is invoked at the runtime, we should keep the error to be raised, as that indicates we're using triton kernel in a scenario triton is not installed

Signed-off-by: Sungjae Lee <[email protected]>
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) October 14, 2025 14:47
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 14, 2025
Copy link
Member

@yewentao256 yewentao256 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work!

@Yikun Yikun added this to the v0.11.1 milestone Oct 16, 2025
@vllm-bot vllm-bot merged commit 00417f4 into vllm-project:main Oct 16, 2025
50 of 52 checks passed
albertoperdomo2 pushed a commit to albertoperdomo2/vllm that referenced this pull request Oct 16, 2025
…26654)

Signed-off-by: Sungjae Lee <[email protected]>
Co-authored-by: Mengqing Cao <[email protected]>
Signed-off-by: Alberto Perdomo <[email protected]>
Zhuul pushed a commit to Zhuul/vllm that referenced this pull request Oct 17, 2025
BoyuanFeng pushed a commit to BoyuanFeng/vllm that referenced this pull request Oct 17, 2025
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants