Skip to content

[Model Runner V2] Do not initialize sampler for non-last PP ranks#36824

Merged
WoosukKwon merged 7 commits intomainfrom
woosuk/mrv2-sampler-last-pp
Mar 12, 2026
Merged

[Model Runner V2] Do not initialize sampler for non-last PP ranks#36824
WoosukKwon merged 7 commits intomainfrom
woosuk/mrv2-sampler-last-pp

Conversation

@WoosukKwon
Copy link
Collaborator

Skip the initialization of Sampler (and a few sample-related classes) for non-last PP ranks or for pooling models.

Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
@WoosukKwon WoosukKwon requested a review from njhill as a code owner March 11, 2026 21:47
@WoosukKwon WoosukKwon added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 11, 2026
@mergify mergify bot added the v1 label Mar 11, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an optimization to skip the initialization of the Sampler and related classes for non-last pipeline parallel ranks and for pooling models. The changes correctly make the initialization of these components conditional, which avoids unnecessary resource allocation. All usages of these potentially uninitialized components are now properly guarded with conditional checks or assertions, ensuring runtime safety. The related modifications in input_batch.py to handle an optional output_bin_counts tensor are also implemented correctly. The changes are logical, well-contained, and effectively achieve the intended optimization.

@mergify
Copy link

mergify bot commented Mar 11, 2026

Hi @WoosukKwon, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy failing?
mypy is run differently in CI. If the failure is related to this check, please use the following command to run it locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10

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.

Nice :)

We've also always done this in V1 .. it had been on my to-do list to fix that too

I think we can similarly conditionally create the pooling_runner in load_model (also no need if not last pp rank)

@WoosukKwon WoosukKwon enabled auto-merge (squash) March 11, 2026 23:23
)
if self.is_pooling_model:
if self.is_pooling_model and self.is_last_pp_rank:
self.pooling_runner = PoolingRunner(self.model)
Copy link
Member

Choose a reason for hiding this comment

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

We may still need to get/store the supported tasks here ... since that's queried from the front-end and the executor returns the result from rank 0.

@WoosukKwon WoosukKwon merged commit 2f8b4ce into main Mar 12, 2026
52 checks passed
@WoosukKwon WoosukKwon deleted the woosuk/mrv2-sampler-last-pp branch March 12, 2026 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants