Skip to content

Conversation

@luccafong
Copy link
Collaborator

@luccafong luccafong commented Oct 16, 2025

Summary: we should not pass the weakref to compilation_config, which include static_forward_context that will holds the pointers to the model layers (e.g. moe, attention), which is dangerous, as this will be passed as config to torch.compile

Differential Revision: D84790018

pytest -v test_sequence_parallelism.py 
pytest -vvv test_async_tp.py
 pytest -vvv test_config.py -k "test_copy_pass"

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 refactors how compilation configuration is passed to the VllmInductorPass. Instead of passing a weak reference to the entire CompilationConfig, it now passes a new SimplifiedCompilationConfig object containing only the necessary fields. This is a great improvement for safety and decoupling, as it avoids passing potentially dangerous pointers (like static_forward_context) into the torch.compile process. However, I've identified a critical type mismatch in the new SimplifiedCompilationConfig dataclass that needs to be addressed.

luccafong pushed a commit to luccafong/vllm that referenced this pull request Oct 16, 2025
…lm-project#27041)

Summary:
Pull Request resolved: vllm-project#27041

we should not pass the weakref to compilation_config, which  include static_forward_context that will holds the pointers to the model layers (e.g. moe, attention), which is dangerous, as this will be passed as config to torch.compile

Test Plan: local tests

Differential Revision: D84790018
luccafong pushed a commit to luccafong/vllm that referenced this pull request Oct 16, 2025
…lm-project#27041)

Summary:
Pull Request resolved: vllm-project#27041

we should not pass the weakref to compilation_config, which  include static_forward_context that will holds the pointers to the model layers (e.g. moe, attention), which is dangerous, as this will be passed as config to torch.compile

Test Plan: local tests

Differential Revision: D84790018

Signed-off-by: Lu Fang <[email protected]>
luccafong pushed a commit to luccafong/vllm that referenced this pull request Oct 16, 2025
…lm-project#27041)

Summary:
Pull Request resolved: vllm-project#27041

we should not pass the weakref to compilation_config, which  include static_forward_context that will holds the pointers to the model layers (e.g. moe, attention), which is dangerous, as this will be passed as config to torch.compile

Test Plan: local tests

Differential Revision: D84790018

Signed-off-by: Lu Fang <[email protected]>
luccafong pushed a commit to luccafong/vllm that referenced this pull request Oct 16, 2025
…lm-project#27041)

Summary:
Pull Request resolved: vllm-project#27041

we should not pass the weakref to compilation_config, which  include static_forward_context that will holds the pointers to the model layers (e.g. moe, attention), which is dangerous, as this will be passed as config to torch.compile

Test Plan: local tests

Differential Revision: D84790018

Signed-off-by: Lu Fang <[email protected]>
…lm-project#27041)

Summary:
Pull Request resolved: vllm-project#27041

we should not pass the weakref to compilation_config, which  include static_forward_context that will holds the pointers to the model layers (e.g. moe, attention), which is dangerous, as this will be passed as config to torch.compile

Test Plan: local tests

Differential Revision: D84790018

Signed-off-by: Lu Fang <[email protected]>
luccafong pushed a commit to luccafong/vllm that referenced this pull request Oct 16, 2025
…lm-project#27041)

Summary:
Pull Request resolved: vllm-project#27041

we should not pass the weakref to compilation_config, which  include static_forward_context that will holds the pointers to the model layers (e.g. moe, attention), which is dangerous, as this will be passed as config to torch.compile

Test Plan: local tests

Differential Revision: D84790018
Signed-off-by: Lu Fang <[email protected]>
@luccafong luccafong changed the title Passing only necessary compilation config to inductor pass config [torch.compile] Passing only necessary compilation config to inductor pass config Oct 16, 2025
use_inductor_graph_partition=config.use_inductor_graph_partition,
compile_sizes=config.compile_sizes,
)
self.pass_config = config.compilation_config.pass_config
Copy link
Collaborator

Choose a reason for hiding this comment

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

later pass_config can be also moved.

Copy link
Collaborator Author

@luccafong luccafong Oct 16, 2025

Choose a reason for hiding this comment

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

I think we can utilize it, but it will introduce duplicated attribute in config level, we can think of how to organize these config better in following PR. @zou3519

Signed-off-by: Lu Fang <[email protected]>
Signed-off-by: Lu Fang <[email protected]>
Copy link
Collaborator

@houseroad houseroad left a comment

Choose a reason for hiding this comment

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

Looks good to me

@luccafong luccafong added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 16, 2025
Signed-off-by: Lu Fang <[email protected]>
@luccafong luccafong enabled auto-merge (squash) October 16, 2025 22:27
@luccafong luccafong merged commit 11ae016 into vllm-project:main Oct 17, 2025
47 checks passed
@github-project-automation github-project-automation bot moved this from To triage to Done in torch.compile integration Oct 17, 2025
Zhuul pushed a commit to Zhuul/vllm that referenced this pull request Oct 17, 2025
@ProExpertProg ProExpertProg linked an issue Oct 18, 2025 that may be closed by this pull request
1 task
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
albertoperdomo2 pushed a commit to albertoperdomo2/vllm that referenced this pull request Oct 23, 2025
… pass config (vllm-project#27041)

Signed-off-by: Lu Fang <[email protected]>
Co-authored-by: Lucia (Lu) Fang <[email protected]>
Signed-off-by: Alberto Perdomo <[email protected]>
alhridoy pushed a commit to alhridoy/vllm that referenced this pull request Oct 24, 2025
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
… pass config (vllm-project#27041)

Signed-off-by: Lu Fang <[email protected]>
Co-authored-by: Lucia (Lu) Fang <[email protected]>
Signed-off-by: xuebwang-amd <[email protected]>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
… pass config (vllm-project#27041)

Signed-off-by: Lu Fang <[email protected]>
Co-authored-by: Lucia (Lu) Fang <[email protected]>
Signed-off-by: xuebwang-amd <[email protected]>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
… pass config (vllm-project#27041)

Signed-off-by: Lu Fang <[email protected]>
Co-authored-by: Lucia (Lu) Fang <[email protected]>
Signed-off-by: 0xrushi <[email protected]>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
… pass config (vllm-project#27041)

Signed-off-by: Lu Fang <[email protected]>
Co-authored-by: Lucia (Lu) Fang <[email protected]>
Signed-off-by: 0xrushi <[email protected]>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
Zhathw pushed a commit to Zhathw/vllm that referenced this pull request Nov 12, 2025
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
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 torch.compile

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: PostGradPassManager should not reference the compilation_config

4 participants