Skip to content

[MRV2] Miscellaneous code structure updates - #60754

Merged
njhill merged 3 commits into
vllm-project:mainfrom
njhill:mrv2-cleanup
Oct 10, 2026
Merged

njhill merged 3 commits into
vllm-project:mainfrom
njhill:mrv2-cleanup

Conversation

@njhill

@njhill njhill commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Various follow-on changes to recently-merged PRs, mostly to minimize/simplify the code in model_runner.py

  • Move _get_drafter_hidden_states to utils file
  • Move kv connector args handling into kv_connector.py
  • Rename dp_sync to dp_sync_state for clarity
  • Remove some redundant .item()s

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@njhill njhill added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 9, 2026
@mergify mergify Bot added speculative-decoding mrv2 Model Runner V2 specific torch.compile labels Oct 9, 2026
@njhill

njhill commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

/ci run

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #93748 for commit 992ae4fb00a9.

njhill added 3 commits October 9, 2026 14:12
Signed-off-by: Nick Hill <nickhill123@gmail.com>
Signed-off-by: Nick Hill <nickhill123@gmail.com>
Signed-off-by: Nick Hill <nickhill123@gmail.com>
@njhill

njhill commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

/ci run

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #93917 for commit 67b5b77b4b8d.

@njhill njhill changed the title [MRV2] Move drafter hidden states util method [MRV2] Miscellaneous code structure updates Oct 9, 2026
Comment on lines -1986 to -1991
connector_kwargs = dict(
scheduler_output=scheduler_output,
request_state_indices=input_batch.idx_mapping,
request_ids=input_batch.req_ids,
num_tokens=input_batch.num_tokens,
)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this also means we avoid creating this dict when a connector isn't enabled

@njhill

njhill commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

/ci retry

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

✅ No failed, timed-out, or expired jobs need retrying: https://buildkite.com/vllm/ci/builds/93917

@njhill
njhill merged commit 7d1d866 into vllm-project:main Oct 10, 2026
202 of 214 checks passed
@njhill
njhill deleted the mrv2-cleanup branch October 10, 2026 00:58
mjbrn added a commit to brntech/vllm that referenced this pull request Oct 10, 2026
…del runner conflict and follow the dp_sync_state rename

- vllm/v1/worker/gpu/model_runner.py: upstream vllm-project#60754 replaced the
  runner's _get_drafter_hidden_states() with the module function
  spec_decode.utils.get_drafter_hidden_states(model, hidden_states).
  Uno's post-AsyncOutput proposal path now calls that function; the
  proposal itself stays in _run_speculator_proposal, whose body equals
  upstream's inline block (watermark preparation, draft workspace lane,
  num_speculative_tokens, blanking past the scheduled K, adaptive
  confidences), and _publish_draft_tokens is unchanged.
- vllm-project#60754 also renamed dp_sync to dp_sync_state in BaseSpeculator.propose,
  ExecuteModelState and the runner. UnoSpeculator.propose,
  _run_speculator_proposal and the Uno runner tests follow the new name,
  so upstream's keyword call in the dummy run reaches Uno's propose.

Signed-off-by: MJ <121681560+mjbrn@users.noreply.github.com>
mjbrn added a commit to brntech/vllm that referenced this pull request Oct 10, 2026
…ropose signature

vllm-project#60754 renamed propose's dp_sync to dp_sync_state, and the merge followed
it in UnoSpeculator.propose. No CPU test would have noticed if it had not:
reverting the parameter to dp_sync left the Uno suite green, because the
runner proposal tests replace propose with a stand-in that accepts any
keyword, and no CPU test reaches the real method by keyword.

The stand-in now binds every call against inspect.signature of the real
UnoSpeculator.propose, so the runner's sample_tokens -> _run_speculator_proposal
path fails on a keyword Uno does not take; and a new test pins
UnoSpeculator.propose's parameter names, order, kinds and defaults to
BaseSpeculator.propose, which the runner's proposal and DP dummy-run call
sites are written against.

Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: MJ <121681560+mjbrn@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dflash mrv2 Model Runner V2 specific ready ONLY add when PR is ready to merge/full CI is needed speculative-decoding torch.compile

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants