jwfromm commented on a change in pull request #6351:
URL: https://github.com/apache/incubator-tvm/pull/6351#discussion_r493654044
##########
File path: python/tvm/autotvm/record.py
##########
@@ -152,7 +152,7 @@ def decode(row, protocol="json"):
tgt, task_name, task_args, task_kwargs = row["input"]
tgt = str(tgt)
if "-target" in tgt:
- logger.warning('"-target" is deprecated, use "-mtriple" instead.')
+ # logger.warning('"-target" is deprecated, use "-mtriple"
instead.')
Review comment:
Is this meant to be commented out in the final PR or did you just remove
it for debugging?
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -331,29 +325,71 @@ def _impl_v1(cls, inputs, attr, params):
return AttrCvt(op_name="instance_norm")(inputs, attr, params)
+def autopad(data, strides, kernel_shape, dilations, ndim, pad_type="constant",
deconv=False):
+ """
+ Perform autopadding with dynamic input shapes
+ """
+ # get attributes as constants
+ strides = _op.const(np.array(strides), dtype="int64")
+ dilated_kernel_shape = _op.const(
+ np.array(
+ [(kernel - 1) * dilation + 1 for kernel, dilation in
zip(kernel_shape, dilations)]
+ ),
+ dtype="int64",
+ )
+ shape = _op.strided_slice(_op.shape_of(data, dtype="int64"), [2], [ndim])
+ # get input shape
+
+ # set up integer constants
+ zero = _op.const(0, dtype="int64")
+ one = _op.const(1, dtype="int64")
+ two = _op.const(2, dtype="int64")
+
+ # Calculate total padding
+ mod = _op.mod(shape, strides)
+
+ left = _op.maximum(dilated_kernel_shape - strides, zero)
+ right = _op.maximum(dilated_kernel_shape - mod, zero)
+
+ total_pad = _op.where(_op.equal(mod, zero), left, right)
+ if deconv:
+ total_pad = _op.const(np.array(kernel_shape), dtype="int64") - one -
total_pad
+
+ # split total padding into before and after
+ pad_before = _op.floor_divide(total_pad, two)
+ pad_after = total_pad - pad_before
+
+ # combine
+ pad = _op.concatenate(
+ [_op.reshape(pad_before, [-1, 1]), _op.reshape(pad_after, [-1, 1])],
axis=1
+ )
+
+ # pad N and C with zeros
+ pad = _op.concatenate([_op.const(np.zeros([2, 2], dtype="int64"),
dtype="int64"), pad], axis=0)
+
+ return _op.nn.pad(data, pad, _op.const(0.0), pad_type)
+
+
class Conv(OnnxOpConverter):
"""Operator converter for Conv."""
@classmethod
def _impl_v1(cls, inputs, attr, params):
# Use shape of input to determine convolution type.
- input_shape = infer_shape(inputs[0])
+ data = inputs[0]
+ input_shape = infer_shape(data)
+ ndim = len(input_shape)
if "auto_pad" in attr:
attr["auto_pad"] = attr["auto_pad"].decode("utf-8")
if attr["auto_pad"] in ("SAME_UPPER", "SAME_LOWER"):
- pad_tuple = []
- for axis in range(len(input_shape) - 2):
- axis_shape = input_shape[2 + axis]
- stride = attr["strides"][axis]
- kernel = attr["kernel_shape"][axis]
- dilation = attr["dilations"][axis]
- dilated_kernel = (kernel - 1) * dilation + 1
- pad = get_pad_pair(axis_shape, dilated_kernel, stride)
- pad_tuple.append(pad)
- pad_tuple = tuple([val for pair in zip(*pad_tuple) for val in
pair])
- attr["pads"] = pad_tuple
+ warning = (
+ "Performing dynamic autopadding on Conv. "
+ + "Conv kernels don't currently support dynamic shapes."
+ )
Review comment:
I'm not sure how often autopad is used in ONNX convolutions, but if its
common I could see this warning showing up in large numbers too frequently.
What do you think about leaving a comment here (and other calls to `autopad`)
instead?
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -347,12 +348,20 @@ def dense_strategy_cpu(attrs, inputs, out_type, target):
def batch_matmul_strategy_cpu(attrs, inputs, out_type, target):
"""batch_matmul x86 strategy"""
strategy = _op.OpStrategy()
- strategy.add_implementation(
- wrap_compute_batch_matmul(topi.x86.batch_matmul),
- wrap_topi_schedule(topi.x86.schedule_batch_matmul),
- name="batch_matmul.x86",
- plevel=10,
- )
+ if is_dynamic(out_type):
+ strategy.add_implementation(
Review comment:
What would the behavior be if instead we only had `if not
is_dynamic(out_type)` to register the x86 schedule? I would think that the
generic strategy would be used even if we dont readd it here.
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -935,16 +975,67 @@ def verify_batch_matmul(a_shape, b_shape):
model = helper.make_model(graph, producer_name="matmul_test")
- for target, ctx in tvm.testing.enabled_targets():
- tvm_out = get_tvm_output(model, [a_array, b_array], target, ctx,
out_np.shape)
+ tvm_out = get_tvm_output_with_vm(model, [a_array, b_array], target, ctx)
+ tvm.testing.assert_allclose(out_np, tvm_out, rtol=1e-5, atol=1e-5)
+
+
+# TODO(mbrookhart): enable cuda once VM supports heterogenous execution
Review comment:
PR #6337 is now merged, should we enable GPU here or are there still
issues?
----------------------------------------------------------------
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:
[email protected]
