Skip to content

Fix free block handling in Inference V2 scheduling - #8503

Open
sj0618 wants to merge 1 commit into
deepspeedai:masterfrom
sj0618:fix/inference-v2-free-blocks
Open

Fix free block handling in Inference V2 scheduling#8503
sj0618 wants to merge 1 commit into
deepspeedai:masterfrom
sj0618:fix/inference-v2-free-blocks

Conversation

@sj0618

@sj0618 sj0618 commented Sep 13, 2026

Copy link
Copy Markdown

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 in maybe_allocate_kv() already reads the same group.

The new tests cover block accounting across two sequences and the insufficient-cache result.

Why

DSStateManager.free_blocks returns one value per cache group. can_schedule() passed that list to get_kv_requirements(), whose current implementations expect an integer. Calls through put(..., 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

.venv/bin/python -m pytest --forked -q -m inference_v2 tests/unit/inference/v2/test_engine_v2.py
2 passed

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.

Signed-off-by: sj0618 <ktx0618@naver.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Mis-typed free_blocks

1 participant