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

Contrib ops for TRT plugins: EfficientNMS and Pyramid ROI Align #9486

Merged

Conversation

wraveane
Copy link
Contributor

@wraveane wraveane commented Oct 21, 2021

Description:
Registers two new contrib ops for TensorRT plugins: EfficientNMS, PyramidROIAlign and multilevelCropAndResizePlugin, which are common operations in object detection networks.

Motivation and Context

  • TensorRT plugins allow fused and efficient execution of common complex operations not natively supported by TensorRT.
  • An ONNX graph that has been modified to use TensorRT plugins ops will normally not run in ORT because it fails graph validation.
  • This PR implements contrib ops for these operations so that ORT accepts them, and the TRT-EP backend can execute them.
  • Additional TensorRT plugins can be registered in the future by following this PR as an example.

@ghost
Copy link

ghost commented Oct 21, 2021

CLA assistant check
All CLA requirements met.

@wraveane wraveane marked this pull request as draft October 21, 2021 23:15
@wraveane wraveane dismissed a stale review via 0879b7d November 17, 2021 20:10
@wraveane wraveane marked this pull request as ready for review November 17, 2021 20:11
@ytaous
Copy link
Contributor

ytaous commented Dec 14, 2021

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline

@ytaous
Copy link
Contributor

ytaous commented Dec 14, 2021

/azp run MacOS NoContribops CI Pipeline, Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 8 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 8 pipeline(s).

@ytaous ytaous added the ep:TensorRT issues related to TensorRT execution provider label Dec 15, 2021
@ytaous
Copy link
Contributor

ytaous commented Dec 15, 2021

seems related - #8461

@wraveane
Copy link
Contributor Author

@ytaous : This PR also needs a small change in the onnx-tensorrt dependency (the NVIDIA ONNX parser), specifically this line:

https://github.com/onnx/onnx-tensorrt/blob/85e79f629fb546a75d61e3027fb259a9529144fe/ModelImporter.cpp#L504-L507

This change was published on onnx-tensorrt with the TRT 8.2 GA release. Should I update the git submodule commit for that dependency as part of this PR already? Otherwise, merging of this PR should probably wait until ORT with TensorRT v8.2 support is fully merged first.

@ytaous
Copy link
Contributor

ytaous commented Dec 15, 2021

@jywu-msft - comments/feedback?

@jywu-msft jywu-msft requested a review from stevenlix December 17, 2021 17:01
@jywu-msft
Copy link
Member

@stevenlix , please review. is this up to date based on latest iteration?

@wraveane
Copy link
Contributor Author

Hello again, I just noticed that the onnx-tensorrt cmake dependency I mentioned before was updated to TRT 8.2 a few days ago already with this commit:

16274be

In that case, this PR should now be ready to merge (after any necessary reviews and such of course).

Copy link
Contributor

@stevenlix stevenlix left a comment

Choose a reason for hiding this comment

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

We see TRT dll issues in windows when use crop and resize plugin. The issue is under investigation and will be addressed in separate PR in the future.

@stevenlix
Copy link
Contributor

/azp run onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ep:TensorRT issues related to TensorRT execution provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants