[GitHub] [incubator-tvm] trevor-m commented on a change in pull request #6905: [TRT][BYOC] handling dynamism in TensorRT to support OD models

[GitBox <gi...@apache.org>]

trevor-m commented on a change in pull request #6905:
URL: https://github.com/apache/incubator-tvm/pull/6905#discussion_r522257464



##########
File path: python/tvm/relay/op/contrib/tensorrt.py
##########
@@ -598,14 +646,26 @@ def strided_slice_annotate_fn(expr):  # pylint: disable=unused-variable
         if batch_dim_begin_modified or batch_dim_end_modified:
             logger.info("strided_slice: can't modify batch dimension.")
             return False
+
     if any([x is not None and x <= 0 for x in attrs.strides]):
         logger.info("strided_slice: stride must be positive")
         return False
+
+    for i in range(0, len(args[0].checked_type.shape)):
+        begin = int(attrs.begin[i])
+        end = (

Review comment:
       We need to take slice mode into account here also

##########
File path: src/relay/backend/contrib/tensorrt/codegen.cc
##########
@@ -133,26 +133,34 @@ class TensorRTJSONSerializer : public backend::contrib::JSONSerializer {
     auto process_slice_index = [](Integer x, int default_value, int dim_value) {
       if (!x.defined()) return default_value;
       int value = x.as<IntImmNode>()->value;
-      if (value < 0) value += dim_value;
+      value = (value < 0 ) ? dim_value + value : value;

Review comment:
       This line is the same as the previous code, can you change it back?

##########
File path: src/runtime/contrib/tensorrt/tensorrt_ops.cc
##########
@@ -944,7 +944,7 @@ class ReduceOpConverter : public TensorRTOpConverter {
 #if TRT_VERSION_GE(5, 1, 5)
 class StridedSliceOpConverter : public TensorRTOpConverter {
  public:
-  StridedSliceOpConverter() : TensorRTOpConverter({kTensor, kWeight, kWeight, kWeight}) {}
+  StridedSliceOpConverter() : TensorRTOpConverter({kTensor}) {} // , kWeight, kWeight, kWeight}) {}

Review comment:
       Remove comment

##########
File path: python/tvm/relay/op/contrib/tensorrt.py
##########
@@ -715,6 +771,34 @@ def conv3d_transpose_annotate_fn(expr):  # pylint: disable=unused-variable
         return False
     return True
 
+_register_external_dynamic_check_func("add", add_annotate_fn)

Review comment:
       I'm not sure if using the wrapper is better than adding a call to check_dynamism to each annotator. With the previous method, the annotator is all in one place with the decorator. Now, we have to remember to call register down here after writing the functions.

##########
File path: tests/python/contrib/test_tensorrt.py
##########
@@ -841,6 +906,39 @@ def get_graph(
     run_and_verify_func(get_graph(strides=(2, 2, 2)))
     run_and_verify_func(get_graph(strides=(2, 2, 2), output_padding=(1, 1, 1)))
 
+def test_tensorrt_ops():

Review comment:
       You can leave these in main

##########
File path: tests/python/contrib/test_tensorrt.py
##########
@@ -219,40 +222,95 @@ def test_tensorrt_not_compatible():
     mod = tvm.IRModule()
     mod["main"] = f
     mod, config = tensorrt.partition_for_tensorrt(mod)
-    with tvm.transform.PassContext(opt_level=3, config={"relay.ext.tensorrt.options": config}):
-        graph, lib, params = relay.build(mod, "cuda")
-    if skip_runtime_test():
-        return
-    mod = graph_runtime.create(graph, lib, ctx=tvm.gpu(0))
-    x_data = np.random.uniform(-1, 1, xshape).astype(dtype)
-    mod.run(x=x_data)
-    results = [mod.get_output(i).asnumpy() for i in range(mod.get_num_outputs())]
+    for mode in ["graph", "vm"]:
+        with tvm.transform.PassContext(opt_level=3, config={"relay.ext.tensorrt.options": config}):
+            exec = relay.create_executor(mode, mod=mod, ctx=tvm.gpu(0), target="cuda")
+            if not skip_runtime_test():
+                results = exec.evaluate()(x_data)
+
 
 
-def test_tensorrt_serialize():
+def test_tensorrt_serialize(data_shape=(1, 3, 224, 224), data_type="float32"):

Review comment:
       I think it would be good to split this into `test_tensorrt_serialize_graph_runtime` and `test_tensorrt_serialize_vm`




----------------------------------------------------------------
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