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.

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

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

Merged
merged 14 commits into from
Feb 14, 2025

Conversation

zhouyu5
Copy link
Contributor

@zhouyu5 zhouyu5 commented Jan 22, 2025

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

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
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.

Signed-off-by: zhouyu5 <yu.zhou@intel.com>
Signed-off-by: zhouyu5 <yu.zhou@intel.com>
Signed-off-by: zhouyu5 <yu.zhou@intel.com>
Signed-off-by: zhouyu5 <yu.zhou@intel.com>
This reverts commit 38d4713.

Signed-off-by: zhouyu5 <yu.zhou@intel.com>
@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?

@zhouyu5
Copy link
Contributor Author

zhouyu5 commented Feb 8, 2025

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

@mgoin Merged now. But the CI still fails, seems it more related to network error or environment error.

image

image

image

Signed-off-by: zhouyu5 <yu.zhou@intel.com>
Signed-off-by: zhouyu5 <yu.zhou@intel.com>
@zhouyu5 zhouyu5 force-pushed the hpu-guided-decoding branch from f386606 to 10003bb Compare February 11, 2025 01:19
Signed-off-by: zhouyu5 <yu.zhou@intel.com>
@zhouyu5
Copy link
Contributor Author

zhouyu5 commented Feb 12, 2025

@mgoin CI passed now, could you help merge it?

@mgoin mgoin added the ready ONLY add when PR is ready to merge/full CI is needed label Feb 12, 2025
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) February 14, 2025 12:36
@simon-mo simon-mo merged commit c9e2d64 into vllm-project:main Feb 14, 2025
42 of 44 checks passed
Sakalya pushed a commit to Sakalya/vllm that referenced this pull request Feb 15, 2025
panf2333 pushed a commit to yottalabsai/vllm that referenced this pull request Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/build ready ONLY add when PR is ready to merge/full CI is needed structured-output
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants