Skip to content

[Bugfix] fix error due to an uninitialized tokenizer when using skip_tokenizer_init with num_scheduler_steps #9276

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
Merged
merged 1 commit into from
Apr 26, 2025
Merged

[Bugfix] fix error due to an uninitialized tokenizer when using skip_tokenizer_init with num_scheduler_steps #9276

merged 1 commit into from
Apr 26, 2025

Conversation

junstar92
Copy link
Contributor

When using skip_tokenizer_init with num_scheduler_steps (scheduling multiple steps), an error occurs due to an uninitialized tokenizer. This PR adds a condition to check tokenizer initialization as single_step.py did.

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

🚀

njhill
njhill reviewed Oct 24, 2024
View reviewed changes
Copy link
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

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

Thanks @junstar92! Could you make the small requested change

@junstar92 junstar92 force-pushed the fix-reference-uninit-tokenizer branch from dd8850f to 4de876a Compare October 25, 2024 00:54
@njhill njhill added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 25, 2024
@junstar92
Copy link
Contributor Author

@njhill How can I fix CI errors ? It doesn't seems like to be caused by this change..

@hmellor
Copy link
Member

hmellor commented Feb 4, 2025

@junstar92 can you try merging the latest main into your branch? (Or rebasing your branch on main, whichever you prefer)

@junstar92 junstar92 force-pushed the fix-reference-uninit-tokenizer branch from 4de876a to 538e2be Compare February 4, 2025 23:20
@junstar92
Copy link
Contributor Author

junstar92 commented Feb 4, 2025 •
edited
Loading

@junstar92 can you try merging the latest main into your branch? (Or rebasing your branch on main, whichever you prefer)

@hmellor I rebased it on the latest main branch. Thank you.

@mergify mergify bot added tpu Related to Google TPUs and removed tpu Related to Google TPUs labels Mar 27, 2025
@mergify mergify bot added the tpu Related to Google TPUs label Apr 9, 2025
@njhill
Copy link
Member

njhill commented Apr 9, 2025

@junstar92 would you mind rebasing again and signing off your commits to satisfy the DCO check: https://github.com/vllm-project/vllm/pull/9276/checks?check_run_id=36686394447 .. thanks!

@junstar92 junstar92 force-pushed the fix-reference-uninit-tokenizer branch from 538e2be to 1496bcd Compare April 9, 2025 14:31
@junstar92
[Bugfix] fix referencing uninitialized tokenizer when using multiple …
af4c6f2 
…step scheduling with skip tokenizer init option

Signed-off-by: changjun.lee <pord7457@gmail.com>
@junstar92 junstar92 force-pushed the fix-reference-uninit-tokenizer branch from 1496bcd to af4c6f2 Compare April 9, 2025 14:33
@junstar92
Copy link
Contributor Author

@njhill I just rebased it and signed off, thanks!

@mergify mergify bot removed the tpu Related to Google TPUs label Apr 9, 2025
@russellb russellb merged commit 10fd1d7 into vllm-project:main Apr 26, 2025
45 checks passed
jikunshang pushed a commit to jikunshang/vllm that referenced this pull request Apr 29, 2025
@junstar92 @dbyoung18
[Bugfix] fix error due to an uninitialized tokenizer when using `skip…
0965571 
…_tokenizer_init` with `num_scheduler_steps` (vllm-project#9276)

Signed-off-by: changjun.lee <pord7457@gmail.com>
lk-chen pushed a commit to lk-chen/vllm that referenced this pull request Apr 29, 2025
@junstar92 @lk-chen
[Bugfix] fix error due to an uninitialized tokenizer when using `skip…
1b017a7 
…_tokenizer_init` with `num_scheduler_steps` (vllm-project#9276)

Signed-off-by: changjun.lee <pord7457@gmail.com>
adobrzyn pushed a commit to HabanaAI/vllm-fork that referenced this pull request Apr 30, 2025
@junstar92 @adobrzyn
[Bugfix] fix error due to an uninitialized tokenizer when using `skip…
3094ea4 
…_tokenizer_init` with `num_scheduler_steps` (vllm-project#9276)

Signed-off-by: changjun.lee <pord7457@gmail.com>
Signed-off-by: Agata Dobrzyniewicz <adobrzyniewicz@habana.ai>
RichardoMrMu pushed a commit to RichardoMrMu/vllm that referenced this pull request May 12, 2025
@junstar92
[Bugfix] fix error due to an uninitialized tokenizer when using `skip…
2e560dd 
…_tokenizer_init` with `num_scheduler_steps` (vllm-project#9276)

Signed-off-by: changjun.lee <pord7457@gmail.com>
Signed-off-by: Mu Huai <tianbowen.tbw@antgroup.com>
@ckhordiasma ckhordiasma mentioned this pull request May 14, 2025
Merged
zzzyq pushed a commit to zzzyq/vllm that referenced this pull request May 24, 2025
@junstar92
[Bugfix] fix error due to an uninitialized tokenizer when using `skip…
8a38db1 
…_tokenizer_init` with `num_scheduler_steps` (vllm-project#9276)

Signed-off-by: changjun.lee <pord7457@gmail.com>
Signed-off-by: Yuqi Zhang <yuqizhang@google.com>
minpeter pushed a commit to minpeter/vllm that referenced this pull request Jun 24, 2025
@junstar92 @minpeter
[Bugfix] fix error due to an uninitialized tokenizer when using `skip…
f3d143b 
…_tokenizer_init` with `num_scheduler_steps` (vllm-project#9276)

Signed-off-by: changjun.lee <pord7457@gmail.com>
Signed-off-by: minpeter <kali2005611@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@njhill njhill njhill approved these changes

Assignees
No one assigned
Labels
ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@junstar92 @hmellor @njhill @russellb