Skip to content

Conversation

@hope51607
Copy link
Contributor

When I run the onnx version mobilenet_v1 from here without freeze_params, I meet an error:

tvm/python/tvm/relay/frontend/onnx.py:6006: in _convert_operator
    sym = convert_map[op_name](inputs, attrs, self._params)
tvm/python/tvm/relay/frontend/onnx.py:2426: in _impl_v1
    data_reshape = _op.reshape(inputs[0], newshape=reshape_shape)

...

        if isinstance(newshape, (tuple, list)):
            tempshape = []
            print("newshape: ", newshape)
            for shape in newshape:
                if isinstance(shape, _expr.IntImm):
                    tempshape.append(shape.value)
                else:
                    try:
                        print("shape: ", shape)
>                       tempshape.append(int(shape))
E                       TypeError: int() argument must be a string, a bytes-like object or a number, not 'Any'
tvm/python/tvm/relay/op/transform.py:311: TypeError

This is caused by Softmax encountering an input with a dynamic shape and using it to construct reshape.
I think simple solution is to directly create Softmax when axis==-1.
I also rewrite the Softmax test to cover dynamic case.

@blackkker @AndrewZhaoLuo @mbrookhart please give it a look.

Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

@hope51607 hope51607 force-pushed the fix_softmax_dynamic branch 2 times, most recently from 9aabbe7 to 4302b50 Compare May 30, 2022 12:45
@hope51607 hope51607 force-pushed the fix_softmax_dynamic branch from 4302b50 to b5f44dc Compare May 30, 2022 13:39
@AndrewZhaoLuo AndrewZhaoLuo self-requested a review May 31, 2022 16:18
Copy link
Contributor

@AndrewZhaoLuo AndrewZhaoLuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, LGTM. Yes this dynamism support stuff can be confusing and inconsistent sometimes. There are failing tests with somewhat scary errors so please take a look at those.

@hope51607
Copy link
Contributor Author

hope51607 commented Jun 1, 2022

Hi @AndrewZhaoLuo, thanks for your reply.

Consider the testing relay graph

def @main(%in: Tensor[(1, 10), float32], %shape: Tensor[(2), int64]) {
  %0 = dyn.reshape(%in, %shape, newshape=[]);
  nn.softmax(%0, axis=1)
}

I found that when the target is cuda, the TIR lowering from softmax contains

allocate(T_softmax_exp: Pointer(warp float32), float32, [any_dim_3]), storage_scope = warp;

which seems to allocate warp memory with any.

I am not sure why the reason behind the problem is that cuda's softmax does not support dynamic shapes? or is it a softmax schedule issue?
I think this issue seems to be out of scope of this PR.
Maybe I should mark skip in these softmax tests with dynamic shapes when target is cuda?

Can anyone give me some hints?
Thanks.

@AndrewZhaoLuo
Copy link
Contributor

I'm not aware of dynamism support for softmax in CUDA but my guess is it's a schedule issue. I'm fine with you marking the dynamic tests as skip with cuda target. Just add a TODO please

@hope51607
Copy link
Contributor Author

@AndrewZhaoLuo got it. I've marked cuda as xfailed and added a TODO comment.

@AndrewZhaoLuo AndrewZhaoLuo merged commit 4f5ab57 into apache:main Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants