[Relay][Frontend][ONNX] operator support: DepthToSpace, SpaceToDepth

[cchung100m]

Hi @zhiics @jroesch @hlu1

Following post at tvm forum, I add the implementation of DepthToSpace operator and SpaceToDepth operator for frontend/onnx.py

I would appreciate if you can review/manage this PR, thank you.

[jwfromm]

Although your changes to the tests removing the hardcoding are definitely in the right direction, I think we should test the actual values instead of just their shapes. To do this, I recommend making a change from your current testing loop:

for target, ctx in ctx_list():
    x = np.random.uniform(size=inshape).astype('int32')
    tvm_out = get_tvm_output(model, x, target, ctx, outshape, 'float32')
tvm.testing.assert_allclose(outshape, tvm_out.shape)

to

for target, ctx in ctx_list():
    x = np.random.uniform(size=inshape).astype('int32')
    tvm_out = get_tvm_output(model, x, target, ctx, outshape, 'float32')
    onnx_out = get_caffe2_output(model, x, 'int32')
    tvm.testing.assert_allclose(onnx_out, tvm_out)

That way we can be sure that both the values and shape produced are correct.

[cchung100m]

Hi @jwfromm

Thanks for the suggestion and I change the code of testing loop.

However, I encounter an issue when running test_space_to_depth() and I would appreciate that if you can help me to solve this issue.

many thanks,

ONNX FATAL: Don't know how to translate op SpaceToDepth
/Users/root/tvm/venv/lib/python3.7/site-packages/caffe2/python/onnx/backend.py:698: UserWarning: This version of onnx-caffe2 targets ONNX operator set version 9, but the model we are trying to import uses version 10.  We will try to import it anyway, but if the model uses operators which had BC-breaking changes in the intervening versions, import will fail.
Traceback (most recent call last):

  File "/Users/root/tvm/tests/python/frontend/onnx/test_forward.py", line 1784, in <module>
    test_space_to_depth()

  File "/Users/root/tvm/tests/python/frontend/onnx/test_forward.py", line 221, in test_space_to_depth
  warnings.warn("This version of onnx-caffe2 targets ONNX operator set version {}, but the model we are trying to import uses version {}.  We will try to import it anyway, but if the model uses operators which had BC-breaking changes in the intervening versions, import will fail.".format(cls._known_opset_version, imp.version))
    verify_space_to_depth((1, 1, 4, 6), (1, 4, 2, 3), 2)

  File "/Users/root/tvm/tests/python/frontend/onnx/test_forward.py", line 212, in verify_space_to_depth
    onnx_out = get_caffe2_output(model, x, 'float32')

  File "/Users/root/tvm/tests/python/frontend/onnx/test_forward.py", line 82, in get_caffe2_output
    prepared_backend = caffe2.python.onnx.backend.prepare(model)

  File "/Users/root/tvm/venv/lib/python3.7/site-packages/caffe2/python/onnx/backend.py", line 712, in prepare
    init_net, predict_net = cls._onnx_model_to_caffe2_net(model, device, opset_version, False)

  File "/Users/root/tvm/venv/lib/python3.7/site-packages/caffe2/python/onnx/backend.py", line 910, in _onnx_model_to_caffe2_net
    raise RuntimeError('ONNX conversion failed')

RuntimeError: ONNX conversion failed

Process finished with exit code 1

[jwfromm]

Hmm, I guess the caffe onnx converter doesn't support SpaceToDepth. I think we really should be using the onnxruntime instead of the caffe2 runtime in the test script, however, the CI image used
doesn't have onnxruntime. @tqchen, what do you think about adding onnxruntime to CI? Is the right way to add it by putting it into docker/install/install_ubuntu_onnx.sh?

[cchung100m]

Hi @jwfromm

Thanks for the prompt reply.

I agree that we can use the onnxruntime to test onnx model, therefore, I add the helper function get_onnxruntime_output to test SpaceToDepth. The code is tested successfully with onnxruntime 0.5.0 in my localhost and it would be great if we can add the onnxruntime in CI pipeline.

[tqchen]

The procedure will be add onnxruntime dep to the docker/install/install_ubuntu_onnx.sh, then we will rebuild and verify the images, merge it to the master before we update the test. if you can send a PR to add onnxruntime installation(with a specific version), I will work on the docker image update incoming week

[jwfromm]

I've added onnxruntime to the docker image in PR #4299. Once it's merged and the CI image is rebuilt the tests in this PR should be able to pass.

[jwfromm]

LGTM! Thanks for putting up with all my feedback. This turned out to a be great PR. I think if you rebase onto the master branch all tests should pass now that PR #4313 is merged.

[cchung100m]

@jwfromm

It is my pleasure to work with you on this PR and I do learn a lot, thanks.

[cchung100m]

Hi @jroesch @soiferj @zhiics

I would appreciate that if you can review/manage this PR, thank you.

[zhiics]

LGTM

[zhiics]

Thanks @cchung100m @jwfromm