Fix: Add None check in step_with_batch_queue for async scheduling#31600
Fix: Add None check in step_with_batch_queue for async scheduling#31600ricky-chaoju wants to merge 1 commit intovllm-project:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a critical AttributeError that occurs when using asynchronous scheduling. The root cause, a missing None check for the result of execute_model in step_with_batch_queue, has been accurately identified. The fix, which involves adding a None check and calling sample_tokens to generate a valid model output, mirrors the existing logic in the step method and is a robust solution to prevent the crash. The implementation is clean, concise, and directly resolves the reported issue.
- Handle None return from execute_model() in step_with_batch_queue() - Mirrors existing pattern in step() method - Fixes AttributeError: 'NoneType' object has no attribute 'sampled_token_ids' - Affects V1 engine with async scheduling enabled (default) Relates to: vllm-project#31588 Signed-off-by: vllm-dev <ricky.chen@infinirc.com>
1c84725 to
68825b6
Compare
|
Thanks @rickychen-infinirc. But this should only ever happen as a secondary error after the execute_model method already raised an exception. Please share the whole log, you should see an earlier root cause error/traceback. I agree that this is still a bug though since the secondary error is misleading. I've opened #31611 to address this. |
|
Closing now that #31611 is merged. |
Summary
Fix
AttributeError: 'NoneType' object has no attribute 'sampled_token_ids'crash in V1 engine when async scheduling is enabled (default).Root Cause
In
step_with_batch_queue()method,execute_model()can returnNoneto signal async scheduling, but there was noNonecheck before passingmodel_outputtoscheduler.update_from_output().The
step()method already handles this correctly (line 364-366), butstep_with_batch_queue()was missing the same check.Fix
Added
Nonecheck instep_with_batch_queue()to callsample_tokens()whenmodel_outputisNone, mirroring the existing pattern instep().Environment
Relates to: #31588