Skip to content
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

[Hardware][Gaudi][Bugfix] Fix error for guided decoding #12317

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[Hardware][Gaudi][Bugfix] Fix error for guided decoding #12317

wants to merge 5 commits into from

Conversation

zhouyu5
Copy link
Contributor

@zhouyu5 zhouyu5 commented Jan 22, 2025 •
edited by github-actions bot
Loading

It fixes the error on Gaudi when enabling guided decoding:

RuntimeError: synNodeCreateWithId failed for node: strided_insert with synStatus 1 [Invalid argument]

After fix, all UT have passed for guided decoding and tool calling, including:

vllm-fork/tests/entrypoints/llm/test_guided_generate.py
vllm-fork/tests/tool_use/*.py

@github-actions GitHub Actions
Copy link

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

@zhouyu5 zhouyu5 force-pushed the hpu-guided-decoding branch from db981c2 to ca1b788 Compare January 23, 2025 09:04
@zhouyu5 zhouyu5 marked this pull request as ready for review January 23, 2025 09:10
@zhouyu5
Copy link
Contributor Author

zhouyu5 commented Jan 23, 2025

/ready

@zhouyu5 zhouyu5 requested a review from mgoin January 23, 2025 09:21
jikunshang
jikunshang reviewed Jan 24, 2025
View reviewed changes
Copy link
Contributor

@jikunshang jikunshang left a comment

Choose a reason for hiding this comment

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

LGTM. possible to add a test in hpu ci?

@mergify mergify bot added the ci/build label Jan 24, 2025
@zhouyu5
Copy link
Contributor Author

zhouyu5 commented Jan 24, 2025

LGTM. possible to add a test in hpu ci?

Added test_guided_generate.py in CI, pls have a check.
It will take around 5 minutes.
@kzawora-intel Could you also have a review? Thanks.

@zhouyu5
initial commit
e575e78 
Signed-off-by: zhouyu5 <[email protected]>
@zhouyu5
add comment
663618c 
Signed-off-by: zhouyu5 <[email protected]>
@zhouyu5
fix format
6038128 
Signed-off-by: zhouyu5 <[email protected]>
@zhouyu5
add ci test
1975785 
Signed-off-by: zhouyu5 <[email protected]>
@zhouyu5
Revert "add ci test"
f5caa08 
This reverts commit 38d4713.

Signed-off-by: zhouyu5 <[email protected]>
@zhouyu5 zhouyu5 force-pushed the hpu-guided-decoding branch from 27e6fa1 to f5caa08 Compare January 26, 2025 06:43
@zhouyu5
Copy link
Contributor Author

zhouyu5 commented Jan 26, 2025

Revert ci test due to out of memory error.

@mgoin
Copy link
Member

mgoin commented Jan 27, 2025

@zhouyu5 can you please merge with main to fix CI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jikunshang jikunshang jikunshang left review comments

@mgoin mgoin mgoin approved these changes

Assignees
No one assigned
Labels
ci/build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zhouyu5 @mgoin @jikunshang