Add unit tests for TensorRT integration and fix some bugs

[Caenorst]

Description

TensorRT integration lacked of unit tests, we instead relied on output comparison of a full network which is not very pertinent, difficult to find the tolerance and not very helpful if it fails.

This PR have two purposes:

1. Add unit test for all operations:
   As we only partition subgraph of at least 2 Ops we always append an identity to each output. we then compare to MXNet, using both TRT FP32 and FP16 computation.
2. Fix a bunch of edge cases bugs that have been exposed by the unit tests:

• NHWC is currently not compatible with TensorRT
• Pooling is currently not compatible with count_include_pad and only compatible with pooling_convention Valid
• FullyConnected without bias cannot be converted to MatMul (as there should be a transpose before) for the moment we decided to remove compatibility, it is quite rare to do FullyConnected without bias anyway
• Concat is not compatible if the concatenation axis is the batch axis
• The dropout have to be only enabled on training mode (act as identity)
• BatchNorm with fix_gamma can have gamma != 1. so we force the value to be 1. when loading it.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

• Changes are complete (i.e. I finished coding on this PR)
• All changes have test coverage:
• Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
• Code is well-documented:
• For user-facing API changes, API doc string has been updated.
• For new C++ functions in header files, their functionalities and arguments are documented.
• To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Comments

• the test of output comparison in a full networks should probably be replaced by an accuracy comparison over a full dataset

[anirudhacharya]

@mxnet-label-bot add [pr-awaiting-review]

[Caenorst]

@KellenSunderland @haohuanw

[KellenSunderland]

Looks like CI caught a few issues. For example http://jenkins.mxnet-ci.amazon-ml.com/blue/organizations/jenkins/mxnet-validation%2Funix-gpu/detail/PR-15399/1/pipeline seems like it should be relevant to this PR. I'll have a look to see if there's anything else that jumps out at me.

[KellenSunderland]

Any idea if TRT actually optimizes this out? I've seen this in a few prod services :-/

[Caenorst]

I believe this should be optimized by ONNX-TRT

[KellenSunderland]

Again, will TensorRT optimize this out? We don't want it at inference time right?

[Caenorst]

Dropout have always been seen as identity function in MXNet-TensorRT integration so I don't see any changement on this, regarding to whether or not identity is actually doing a copy or not I'm not quite sure, here is the onnx-tensorrt conversion: https://github.com/onnx/onnx-tensorrt/blob/0ab159579551cabfa05fd66f338357f116e96835/trt_utils.hpp#L169-L180

[KellenSunderland]

Ok, non-blocking comment for this PR. I'm just thinking about adding a warning in the future if people are using TRT with operations that don't make sense at inference time (Dropout, Ident, Empty Concats or Copies, etc.)

[KellenSunderland]

Few small changes requested. Looks like CI caught a few issues as well.

[karan6181]

@Caenorst Could you please address the review comments? Thanks!

[piyushghai]

@Caenorst Gentle ping...

[Caenorst]

I don't understand the error on windows-gpu, it doesn't seems related to my modifications...

[Caenorst]

@KellenSunderland can we merge it ? (I did a bunch of modifications since the last review that you may wanna review too)