[BugFix] fix VLLM_USE_STANDALONE_COMPILE=0#38015
Conversation
I broke this in one of the refactors, this fixes it and adds some testing Signed-off-by: Richard Zou <zou3519@gmail.com>
zou3519
requested review from
BoyuanFeng,
ProExpertProg and
youkaichao
as code owners
![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 introduces a new test to validate the correctness of standalone compilation and implements a fix in vllm/compilation/compiler_interface.py to resolve a FakeTensorMode mismatch when compile_fx is used within Dynamo's tracing context. The review feedback highlights that the implemented fix relies on a private PyTorch API, torch._guards._TLS.tracing_context, which could lead to fragility and future breakage. It is recommended to add a comment explaining this dependency and to consider filing an issue with PyTorch to request a public API for this functionality.
| saved_tracing_context = torch._guards.TracingContext.try_get() | ||
| if saved_tracing_context is not None: | ||
| torch._guards._TLS.tracing_context = None | ||
|
|
||
| def _restore_tracing_context(): | ||
| torch._guards._TLS.tracing_context = saved_tracing_context | ||
|
|
||
| stack.callback(_restore_tracing_context) |
There was a problem hiding this comment.
This fix relies on torch._guards._TLS.tracing_context, which is a private, undocumented PyTorch API. This makes the code fragile and likely to break in future PyTorch versions.
To improve maintainability, please add a comment to this block explaining that this uses a private PyTorch API and could break in the future. It would also be beneficial to file an issue with PyTorch to request a public API for this functionality and link it in the comment.
References
- Avoid using private, undocumented APIs from third-party libraries, as they are not part of the public contract and can change or be removed without notice, leading to future breakage.
There was a problem hiding this comment.
we're going to deprecate and delete this path (USE_STANDALONE_COMPILE=0) so I'm not worried about it
zhxchen17
left a comment
zhxchen17
left a comment
There was a problem hiding this comment.
Looks good to me. but is InductorAdapter still actively used by default? I didn't remember hitting this one in most testing
|
StandaloneInductorAdaptor is used by default, InductorAdaptor hasn't been used by default in a while (which is why it has bitrotted). I wanted this to work because I was experimenting with it |
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: Richard Zou <zou3519@gmail.com> Signed-off-by: Michel Belleau <michel.belleau@malaiwah.com>
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: Richard Zou <zou3519@gmail.com> Signed-off-by: Monishver Chandrasekaran <monishverchandrasekaran@gmail.com>
Signed-off-by: Richard Zou <zou3519@gmail.com> Signed-off-by: Nithin Chalapathi <nithin.ch10@gmail.com>
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: Richard Zou <zou3519@gmail.com> Signed-off-by: Vinay Damodaran <vrdn@hey.com>
3 participants
I broke this in one of the refactors, this fixes it and adds some testing