TensorRT icon indicating copy to clipboard operation
TensorRT copied to clipboard

feat: add a pre-AOT lowering pass to remove detach ops

Open zewenli98 opened this issue 1 year ago • 6 comments

Description

Added a pre-AOT lowering pass to remove detach ops

Fixes #2657

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • [x] My code follows the style guidelines of this project (You can use the linters)
  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas and hacks
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have added tests to verify my fix or my feature
  • [ ] New and existing unit tests pass locally with my changes
  • [x] I have added the relevant labels to my PR in so that relevant reviewers are notified

zewenli98 avatar Apr 16 '24 22:04 zewenli98

Overall looks good to me. Is there any use case for which this lowering pass has been added?

apbose avatar Apr 17 '24 01:04 apbose

Overall looks good to me. Is there any use case for which this lowering pass has been added?

Yes, in the vLLM, without this lowering pass, it causes some errors during compilation.

zewenli98 avatar Apr 17 '24 22:04 zewenli98

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.

Where can I find your implementation? The link seems not including your change.

zewenli98 avatar Apr 19 '24 06:04 zewenli98

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.

Where can I find your implementation? The link seems not including your change.

My bad, here's the correct link : https://github.com/pytorch/TensorRT/pull/2763

peri044 avatar Apr 22 '24 23:04 peri044

Hi @peri044 I checked your PR and found you made a lot of changes like pre_export_lowering and post_lowering with the help of PassManager. I will rebase on your PR after it gets merged.

Besides, I noticed that _pretraced_backend() in backends.py and compile() in _compiler.py are pretty similar. Why do we need the separate functions?

zewenli98 avatar Apr 25 '24 00:04 zewenli98

Hi @peri044 I checked your PR and found you made a lot of changes like pre_export_lowering and post_lowering with the help of PassManager. I will rebase on your PR after it gets merged.

Besides, I noticed that _pretraced_backend() in backends.py and compile() in _compiler.py are pretty similar. Why do we need the separate functions?

We use the same function now https://github.com/pytorch/TensorRT/pull/2763. Closing this PR now

peri044 avatar May 17 '24 17:05 peri044