Skip to content
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

[TENSORRT] Improvements and fixes for TensorRT #11203

Merged
merged 1 commit into from
May 10, 2022

Conversation

mbaret
Copy link
Contributor

@mbaret mbaret commented May 3, 2022

A number of small fixes and refactors to improve the robustness of the TensorRT integration.

Co-authored-by: Mark Shields mbs@octoml.ai

A number of small fixes and refactors to improve the robustness of
the TensorRT integration.

Co-authored-by: Mark Shields <mbs@octoml.ai>
@mbaret
Copy link
Contributor Author

mbaret commented May 4, 2022

cc @mbs-octoml @mikepapadim

@mbs-octoml
Copy link
Contributor

The background on the bits I fiddled with:

  • Finish the transition from operator predicate based to pattern based for the partition_for_tensorrt function. The current state in main is broken.
  • Remove choice of operator vs predicate method since they are indistinguishable from the outside.
  • Though most operators can be translated looking only at the operator call, nn.batch_norm can only be translated in the form of a sub-graph nn.batch_norm(...).0. So switch the translation to follow the standard composite style.

Copy link
Contributor

@mbs-octoml mbs-octoml left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

I wonder if we should warn for the conv3d attributes you found trigger numerical inaccuracy? Say in the predicate?

@mbaret
Copy link
Contributor Author

mbaret commented May 4, 2022

Unfortunately, the attributes aren't particularly well-defined. For example, in that specific test it was failing with channels between 28 and 45. I think so long as TensorRT is selecting strategies with different numerical behaviour based on tuning we won't be able to give a sane warning here.

@mbs-octoml
Copy link
Contributor

Ack. We do what we can.

@mikepapadim
Copy link
Contributor

LGTM, thanks

Copy link
Contributor

@jwfromm jwfromm left a comment

Choose a reason for hiding this comment

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

Thanks for these improvements @mbaret, @mikepapadim, and @mbs-octoml!

@jwfromm jwfromm merged commit be2ae94 into apache:main May 10, 2022
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request May 16, 2022
A number of small fixes and refactors to improve the robustness of
the TensorRT integration.

Co-authored-by: Mark Shields <mbs@octoml.ai>

Co-authored-by: Mark Shields <mbs@octoml.ai>
shtinsa pushed a commit to Deelvin/tvm that referenced this pull request May 17, 2022
A number of small fixes and refactors to improve the robustness of
the TensorRT integration.

Co-authored-by: Mark Shields <mbs@octoml.ai>

Co-authored-by: Mark Shields <mbs@octoml.ai>
shingjan pushed a commit to shingjan/tvm that referenced this pull request May 17, 2022
A number of small fixes and refactors to improve the robustness of
the TensorRT integration.

Co-authored-by: Mark Shields <mbs@octoml.ai>

Co-authored-by: Mark Shields <mbs@octoml.ai>
SebastianBoblest pushed a commit to SebastianBoblest/tvm that referenced this pull request May 27, 2022
A number of small fixes and refactors to improve the robustness of
the TensorRT integration.

Co-authored-by: Mark Shields <mbs@octoml.ai>

Co-authored-by: Mark Shields <mbs@octoml.ai>
@tiandiao123
Copy link
Contributor

nice job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants