[BugFix] Patch inductor partitioning logic#26735
[BugFix] Patch inductor partitioning logic#26735ProExpertProg merged 2 commits intovllm-project:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a monkey-patch for PyTorch Inductor's partitioning logic to address a bug in torch version 2.9. The patch is applied conditionally based on the torch version. My review focuses on a misleading comment within the patch that contradicts the code's behavior, which could impact future maintainability. I've suggested a correction to make the comment accurate.
vllm/env_override.py
Outdated
| # Copied from torch._inductor.scheduler.Scheduler.should_partition. Patches | ||
| # [this code](https://github.com/pytorch/pytorch/blob/ecb53078faf86ca1b33277df33b82985675bb011/torch/_inductor/scheduler.py#L4712-L4724) | ||
| # so that we always return True. |
There was a problem hiding this comment.
The comment on line 31, so that we always return True, contradicts the function's implementation which can return False (as seen on line 96). This is misleading and could cause confusion for future maintenance. Please update the comment to accurately describe the patch's purpose, which appears to be reverting to a previous, correct behavior of should_partition.
| # Copied from torch._inductor.scheduler.Scheduler.should_partition. Patches | |
| # [this code](https://github.com/pytorch/pytorch/blob/ecb53078faf86ca1b33277df33b82985675bb011/torch/_inductor/scheduler.py#L4712-L4724) | |
| # so that we always return True. | |
| # This is a patched version of torch._inductor.scheduler.Scheduler.should_partition | |
| # that reverts to a prior implementation to fix a regression. | |
| # See: https://github.com/pytorch/pytorch/blob/ecb53078faf86ca1b33277df33b82985675bb011/torch/_inductor/scheduler.py#L4712-L4724 |
f8fd05b to
f169d0e
Compare
vllm/env_override.py
Outdated
| if is_torch_equal_or_newer("2.9.0.dev"): | ||
| GraphLowering._update_scheduler = _update_scheduler_patched |
There was a problem hiding this comment.
I guess we should fix this as soon as we can in PyTorch and change this to just check torch==2.9.0, because someone can change GraphLowering._update_scheduler on PyTorch main :/. I'll remember to loop back to this post-PTC.
There was a problem hiding this comment.
+1 for patching only for torch==2.9.0
zou3519
left a comment
There was a problem hiding this comment.
lgtm, will let you two figure out the ordering of merging the PRs.
f169d0e to
dc3da2a
Compare
|
Should we have a dedicated folder for pt2.9 monkey patch? |
Signed-off-by: angelayi <yiangela7@gmail.com>
Head branch was pushed to by a user without write access
dc3da2a to
0ba846b
Compare
Signed-off-by: angelayi <yiangela7@gmail.com>
|
I think moving the import might just postpone the ci failure to the 2.9 PR but at least it'll resolve other tests there 👍 |
| if version.parse(str(torch.__version__)) == version.parse("2.9.0"): | ||
| from torch._inductor.graph import GraphLowering | ||
|
|
||
| GraphLowering._update_scheduler = _update_scheduler_patched |
There was a problem hiding this comment.
would it work if we just use Scheduler.should_partition = should_partition_patched instead of _update_scheduler_patched?
| self.scheduler = Scheduler(self.operations) | ||
|
|
||
|
|
||
| if version.parse(str(torch.__version__)) == version.parse("2.9.0"): |
There was a problem hiding this comment.
We can fix in your PR, for main this is a no-op, in #26738 I manually made sure to use your approach
commit a4ee300 Author: angelayi <yiangela7@gmail.com> Date: Tue Oct 14 19:19:25 2025 -0700 test moving import Signed-off-by: angelayi <yiangela7@gmail.com> commit 0ba846b Author: angelayi <yiangela7@gmail.com> Date: Mon Oct 13 13:36:43 2025 -0700 [BugFix] Patch inductor partitioning logic Signed-off-by: angelayi <yiangela7@gmail.com> Signed-off-by: ProExpertProg <lgovedic@redhat.com>
Signed-off-by: angelayi <yiangela7@gmail.com> Signed-off-by: bbartels <benjamin@bartels.dev>
Signed-off-by: angelayi <yiangela7@gmail.com>
Signed-off-by: angelayi <yiangela7@gmail.com>
Signed-off-by: angelayi <yiangela7@gmail.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
Signed-off-by: angelayi <yiangela7@gmail.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
Signed-off-by: angelayi <yiangela7@gmail.com>
Signed-off-by: angelayi <yiangela7@gmail.com>

Purpose
Monkey-patches inductor 2.9 code to fix #26678
Test Plan
cc @zou3519 @ProExpertProg @BoyuanFeng