Skip to content

[Bugfix] Fix triton and minicpm patch #690

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
Closed
wants to merge 1 commit into from
Closed

[Bugfix] Fix triton and minicpm patch #690

wants to merge 1 commit into from

Conversation

MengqingCao
Copy link
Collaborator

@MengqingCao MengqingCao commented Apr 28, 2025 •
edited
Loading

What this PR does / why we need it?

Triton patch does not work as expected. Adding TritonPlaceholder makes vllm mistakenly think that triton is installed and enter the wrong branch. vllm does not expose triton _utils so we cannot patch HAS_TRITON correctly.

Python 3.10.16 (main, Dec 11 2024, 16:18:56) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import vllm
/home/xxx/miniconda3/envs/atb/lib/python3.10/site-packages/torch_npu/__init__.py:289: UserWarning: On the interactive interface, the value of TASK_QUEUE_ENABLE is set to 0 by default.                      Do not set it to 1 to prevent some unknown errors
  warnings.warn("On the interactive interface, the value of TASK_QUEUE_ENABLE is set to 0 by default. \
INFO 04-28 01:40:59 [__init__.py:30] Available plugins for group vllm.platform_plugins:
INFO 04-28 01:40:59 [__init__.py:32] name=ascend, value=vllm_ascend:register
INFO 04-28 01:40:59 [__init__.py:34] all available plugins for group vllm.platform_plugins will be loaded.
INFO 04-28 01:40:59 [__init__.py:36] set environment variable VLLM_PLUGINS to control which plugins to load.
INFO 04-28 01:40:59 [__init__.py:44] plugin ascend loaded.
INFO 04-28 01:40:59 [__init__.py:230] Platform plugin ascend is activated
WARNING:root:Warning: Failed to register custom ops, all custom ops will be disabled
>>> vllm.t
vllm.test_utils          vllm.third_party         vllm.tracing             vllm.transformers_utils

Thus I think it's better not to contain minicpm in v0.8.4, and removing the triton patch.

Does this PR introduce any user-facing change?

couldnot use minicpm in v0.8.4

How was this patch tested?

CI passed with new existing test.

@MengqingCao
Copy link
Collaborator Author

@mengwei805 Locally tested that #636 could work after this pr, plz rebase your code when this pr is merged

@mengwei805
Copy link
Collaborator

@mengwei805 Locally tested that #636 could work after this pr, plz rebase your code when this pr is merged

thx

@wangxiyuan wangxiyuan closed this Apr 28, 2025
@Yikun Yikun mentioned this pull request May 5, 2025
Merged
Yikun added a commit that referenced this pull request May 5, 2025
@Yikun
Re-patch TritonPlaceholder on main to make CI happy (#753)
d7e1110 
### What this PR does / why we need it?
Re-patch TritonPlaceholder on main to make CI happy
- Add triton patch back until
vllm-project/vllm#17446 resolved
- Move patch_main before patch_common to resolve minicpm triton import
issue
- Add `0.8.5` and `0.8.5.post1` to make patch work on 0.8.5 all versions

Related:
- #704
- #690

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
All CI passed include main

Signed-off-by: Yikun Jiang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MengqingCao @mengwei805 @wangxiyuan