Fix free block handling in Inference V2 scheduling - #8503
Conversation
Signed-off-by: sj0618 <ktx0618@naver.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6389374b33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| def _engine(free_blocks, get_kv_requirements): | ||
| engine = InferenceEngineV2.__new__(InferenceEngineV2) |
There was a problem hiding this comment.
Cover the inference path with a device-backed integration test
This changes the scheduling invoked by put(..., do_checks=True), but _engine() constructs an uninitialized engine and substitutes SimpleNamespace collaborators, so neither added test exercises DSStateManager.free_blocks or a model's get_kv_requirements() during an actual inference request. A backend or model integration mismatch in the repaired production path would therefore remain undetected; add and execute a minimal device-backed put test.
AGENTS.md reference: AGENTS.md:L35-L36
Useful? React with 👍 / 👎.
What changed
InferenceEngineV2.can_schedule()now reads the free block count from the single KV cache group used by current model implementations. The allocation path inmaybe_allocate_kv()already reads the same group.The new tests cover block accounting across two sequences and the insufficient-cache result.
Why
DSStateManager.free_blocksreturns one value per cache group.can_schedule()passed that list toget_kv_requirements(), whose current implementations expect an integer. Calls throughput(..., do_checks=True)therefore failed during scheduling with a list-to-integer comparison error.Supporting multiple KV cache groups requires a separate scheduling contract and is outside this fix.
Fixes #5407
Tests
All pre-commit hooks passed for the changed files.
The full Inference V2 suite was not used as validation on macOS. Existing allocator and sharding tests require the CUDA execution path and crash under MPS. The added scheduling tests do not initialize CUDA or load a model.