New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add a pre-AOT lowering pass to remove detach ops #2756
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - if possible, could a model with an explicit detach()
call be added as a test case?
Overall looks good to me. Is there any use case for which this lowering pass has been added? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've implemented your fix (refactored it a bit) as a part of https://github.com/pytorch/TensorRT/pull/2756/files. Can you take a look and make these changes?
We need to add this pass to both torch.compile and torch.export as well.
Yes, in the vLLM, without this lowering pass, it causes some errors during compilation. |
Where can I find your implementation? The link seems not including your change. |
My bad, here's the correct link : #2763 |
b052e2a
to
e0b7ce7
Compare
Hi @peri044 I checked your PR and found you made a lot of changes like Besides, I noticed that |
We use the same function now #2763. Closing this PR now |
Description
Added a pre-AOT lowering pass to remove
detach
opsFixes #2657
Type of change
Checklist: