[torch.compile] fix simple inductor graph partition test #27050
Conversation
Signed-off-by: Boyuan Feng <boyuan@meta.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a failing test for inductor graph partitioning by ensuring that splitting_ops is properly configured. The test was failing because a recent change made inductor respect splitting_ops, and the test was passing an empty list, which disabled partitioning. My main feedback is to remove an outdated and misleading comment in the test code that contradicts the new behavior.
| monkeypatch.setenv("VLLM_DISABLE_COMPILE_CACHE", "1") | ||
|
|
||
| _run_simple_model( | ||
| # Inductor graph partition automatically resets splitting_ops to an empty list |
There was a problem hiding this comment.
This comment appears to be outdated and incorrect. The pull request description explains that inductor partitioning now respects splitting_ops, and the test was failing with splitting_ops=[] because it prevented partitioning. This comment claims the opposite, that splitting_ops is automatically reset to an empty list. To avoid confusion for future developers, this comment should be removed.
There was a problem hiding this comment.
Can you remove the comment?
ProExpertProg
added this to the
vllm==v0.12.0/torch==2.9.0 compilation improvements milestone
commit 41590e7 Author: Boyuan Feng <boyuan@meta.com> Date: Thu Oct 16 13:25:18 2025 -0700 nit Signed-off-by: Boyuan Feng <boyuan@meta.com> commit 8393288 Author: Boyuan Feng <boyuan@meta.com> Date: Thu Oct 16 13:04:14 2025 -0700 fix simple inductor graph partition test Signed-off-by: Boyuan Feng <boyuan@meta.com> Signed-off-by: ProExpertProg <lgovedic@redhat.com>
ProExpertProg
added
the
ready
|
Passed tests in CI, merged! |
ProExpertProg
changed the title
…t#27050) Signed-off-by: Boyuan Feng <boyuan@meta.com>
…t#27050) Signed-off-by: Boyuan Feng <boyuan@meta.com>
…t#27050) Signed-off-by: Boyuan Feng <boyuan@meta.com> Signed-off-by: Alberto Perdomo <aperdomo@redhat.com>
…t#27050) Signed-off-by: Boyuan Feng <boyuan@meta.com>
…t#27050) Signed-off-by: Boyuan Feng <boyuan@meta.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
…t#27050) Signed-off-by: Boyuan Feng <boyuan@meta.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
…t#27050) Signed-off-by: Boyuan Feng <boyuan@meta.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
…t#27050) Signed-off-by: Boyuan Feng <boyuan@meta.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
2 participants
This PR fixes a inductor graph partition test.
The error happened since silly attention is NOT partitioned. Why? Because #25845 lets inductor partition respect splitting_ops and the test
splitting_ops=[]says don't partition on any ops.