Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix stride default value None in torch.nn.functional.avg_pool #4984

Merged
merged 5 commits into from
Mar 7, 2020

Conversation

pyjhzwh
Copy link
Contributor

@pyjhzwh pyjhzwh commented Mar 4, 2020

The default value of stride in nn.functional.avg_pool is set as None,
https://github.com/apache/incubator-tvm/blob/5a0f39b5481a30a2eec49e27cbc17a722bd6ee6a/python/tvm/relay/frontend/pytorch.py#L461
It will cause TVMError: Check failed: ObjectTypeChecker<TObjectRef>: :Check(ptr): Expect relay.Expr but get Array since inputs[2] is empty.
Fix it by detecting None type of stride in tvm frontend pytorch.

@@ -918,7 +921,7 @@ def _get_constant(node):

def _get_operator_nodes(nodes):
""" Returns torch IR nodes that need conversion to Relay """
ops = {}
ops = []
Copy link
Member

@masahi masahi Mar 4, 2020

Choose a reason for hiding this comment

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

You should not have this change in this pr. Fetch and rebase against master.

@masahi
Copy link
Member

masahi commented Mar 4, 2020

if inputs[2]:
strides = _infer_shape(inputs[2])
else:
strides = pool_size
Copy link
Member

Choose a reason for hiding this comment

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

is this correct? we should use the default strides

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/pytorch/pytorch/blob/17a5c677963dc3ecb7ff505585ed15eadaaf74ef/torch/nn/functional.py#L269-L270
According to the description of strides above, for avg_pool2d. Stride's default value kernel_size, which should have the same size as pool_size here

Copy link
Member

Choose a reason for hiding this comment

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

ok

@masahi
Copy link
Member

masahi commented Mar 5, 2020

Let's wait until the commit author issue is resolved https://discuss.tvm.ai/t/github-issue-the-commit-author-is-wrong-since-today/5880/

@masahi masahi self-assigned this Mar 6, 2020
@masahi
Copy link
Member

masahi commented Mar 7, 2020

It seems the github bug was resolved, I'll try merge it.

@masahi masahi merged commit de0869d into apache:master Mar 7, 2020
@masahi
Copy link
Member

masahi commented Mar 7, 2020

Looks like it is ok.

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Apr 16, 2020
…#4984)

* fix unordered dictionary problem for python version 3.5

* modify style

* default value of stride in torch.nn.functional.avg_pool is None

* delete prev modifications

* add testcase for nn.functional.avg_pool2d
zhiics pushed a commit to neo-ai/tvm that referenced this pull request Apr 17, 2020
…#4984)

* fix unordered dictionary problem for python version 3.5

* modify style

* default value of stride in torch.nn.functional.avg_pool is None

* delete prev modifications

* add testcase for nn.functional.avg_pool2d
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