You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/12/03 19:35:16 UTC
[GitHub] [tvm] trevor-m opened a new pull request #7026: [BYOC][TRT] Support batch norm for all ranks <=5, and all axes
trevor-m opened a new pull request #7026:
URL: https://github.com/apache/tvm/pull/7026
Previous batch norm only supported rank 4 inputs with axis 1 or 3. Now we support input ranks and axes 1-5.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch commented on a change in pull request #7026: [BYOC][TRT] Support batch norm for all ranks <=5, and all axes
Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #7026:
URL: https://github.com/apache/tvm/pull/7026#discussion_r537154472
##########
File path: src/runtime/contrib/tensorrt/tensorrt_ops.cc
##########
@@ -386,8 +386,35 @@ class BatchNormOpConverter : public TensorRTOpConverter {
const int axis = std::stoi(params->node.GetAttr<std::vector<std::string>>("axis")[0]);
const bool scale = std::stoi(params->node.GetAttr<std::vector<std::string>>("scale")[0]);
const bool center = std::stoi(params->node.GetAttr<std::vector<std::string>>("center")[0]);
- ICHECK(axis == 1 || axis == 3);
- const bool need_transpose = axis == 3;
+ auto input_dims = TrtDimsToVector(input->getDimensions());
+ const size_t min_rank = TRT_HAS_IMPLICIT_BATCH(params) ? 3 : 4;
+ const size_t max_rank = TRT_HAS_IMPLICIT_BATCH(params) ? 4 : 5;
+ ICHECK_LE(input_dims.size(), max_rank);
Review comment:
Could you convert these checks to use `Diagnostic` instead of generating an assertion, we should strive to replace most of these with end-user readable errors.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] anijain2305 commented on a change in pull request #7026: [BYOC][TRT] Support batch norm for all ranks <=5, and all axes
Posted by GitBox <gi...@apache.org>.
anijain2305 commented on a change in pull request #7026:
URL: https://github.com/apache/tvm/pull/7026#discussion_r535541583
##########
File path: python/tvm/relay/op/contrib/tensorrt.py
##########
@@ -341,6 +341,11 @@ def batch_norm_annotate_fn(expr): # pylint: disable=unused-variable
if any([x.checked_type.dtype != "float32" for x in args]):
logger.info("Only float32 inputs are supported for TensorRT.")
return False
+ if len(args[0].checked_type.shape) == 5 and get_tensorrt_version() < (6, 0, 1):
+ logger.info("nn.batch_norm: TensorRT 6.0.1 or higher is required for rank 5 inputs.")
Review comment:
MIssing `return False`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] trevor-m commented on pull request #7026: [BYOC][TRT] Support batch norm for all ranks <=5, and all axes
Posted by GitBox <gi...@apache.org>.
trevor-m commented on pull request #7026:
URL: https://github.com/apache/tvm/pull/7026#issuecomment-738275838
All tests in test_tensorrt.py passed locally
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] zhiics commented on pull request #7026: [BYOC][TRT] Support batch norm for all ranks <=5, and all axes
Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #7026:
URL: https://github.com/apache/tvm/pull/7026#issuecomment-740791470
Thanks @trevor-m @anijain2305 @jroesch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] trevor-m commented on a change in pull request #7026: [BYOC][TRT] Support batch norm for all ranks <=5, and all axes
Posted by GitBox <gi...@apache.org>.
trevor-m commented on a change in pull request #7026:
URL: https://github.com/apache/tvm/pull/7026#discussion_r537729585
##########
File path: src/runtime/contrib/tensorrt/tensorrt_ops.cc
##########
@@ -386,8 +386,35 @@ class BatchNormOpConverter : public TensorRTOpConverter {
const int axis = std::stoi(params->node.GetAttr<std::vector<std::string>>("axis")[0]);
const bool scale = std::stoi(params->node.GetAttr<std::vector<std::string>>("scale")[0]);
const bool center = std::stoi(params->node.GetAttr<std::vector<std::string>>("center")[0]);
- ICHECK(axis == 1 || axis == 3);
- const bool need_transpose = axis == 3;
+ auto input_dims = TrtDimsToVector(input->getDimensions());
+ const size_t min_rank = TRT_HAS_IMPLICIT_BATCH(params) ? 3 : 4;
+ const size_t max_rank = TRT_HAS_IMPLICIT_BATCH(params) ? 4 : 5;
+ ICHECK_LE(input_dims.size(), max_rank);
Review comment:
Hi @jroesch, thanks for reviewing!
These checks are more for sanity checking, since the annotation functions in python/tvm/relay/op/contrib/tensorrt.py will filter out the unsupported ops before they ever get to this code. I don't expect users to ever see these.
Anyway, I can make a separate PR to port all of the ICHECK to Diagnostics.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] trevor-m commented on a change in pull request #7026: [BYOC][TRT] Support batch norm for all ranks <=5, and all axes
Posted by GitBox <gi...@apache.org>.
trevor-m commented on a change in pull request #7026:
URL: https://github.com/apache/tvm/pull/7026#discussion_r537729585
##########
File path: src/runtime/contrib/tensorrt/tensorrt_ops.cc
##########
@@ -386,8 +386,35 @@ class BatchNormOpConverter : public TensorRTOpConverter {
const int axis = std::stoi(params->node.GetAttr<std::vector<std::string>>("axis")[0]);
const bool scale = std::stoi(params->node.GetAttr<std::vector<std::string>>("scale")[0]);
const bool center = std::stoi(params->node.GetAttr<std::vector<std::string>>("center")[0]);
- ICHECK(axis == 1 || axis == 3);
- const bool need_transpose = axis == 3;
+ auto input_dims = TrtDimsToVector(input->getDimensions());
+ const size_t min_rank = TRT_HAS_IMPLICIT_BATCH(params) ? 3 : 4;
+ const size_t max_rank = TRT_HAS_IMPLICIT_BATCH(params) ? 4 : 5;
+ ICHECK_LE(input_dims.size(), max_rank);
Review comment:
Hi @jroesch, thanks for reviewing!
These checks are more for sanity checking, since the annotation functions in python/tvm/relay/op/contrib/tensorrt.py will filter out the unsupported ops before they ever get to this code. I don't expect users to ever see these.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] zhiics merged pull request #7026: [BYOC][TRT] Support batch norm for all ranks <=5, and all axes
Posted by GitBox <gi...@apache.org>.
zhiics merged pull request #7026:
URL: https://github.com/apache/tvm/pull/7026
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org