Fix shape inference pass #14153

ptrendx · 2019-02-14T01:17:46Z

Description

This PR fixes shape inference, which currently results in wrong result for some cases.

An example problematic case (as it works in current version of MXNet, before applying this change):
If I do

import mxnet as mx

data = mx.sym.Variable('data', shape=(1,0,512,512))
weight = mx.sym.Variable('weight')
cdata = mx.sym.cast(data, dtype='float16')
cweight = mx.sym.cast(weight, dtype='float16')
test = mx.sym.Convolution(data=cdata, weight=cweight,layout='NCHW', pad=(3, 3), num_filter=64, stride=(2, 2), no_bias=True, dilate=(1, 1), kernel=(7, 7), num_group=1)

print(test.infer_shape_partial())

I get expected result:

([(1, 0, 512, 512), (64, 0, 7, 7)], [(1, 64, 256, 256)], [])

but when I change H and W dimensions in the shape to 0s,

import mxnet as mx

data = mx.sym.Variable('data', shape=(1,0,0,0))
weight = mx.sym.Variable('weight')
cdata = mx.sym.cast(data, dtype='float16')
cweight = mx.sym.cast(weight, dtype='float16')
test = mx.sym.Convolution(data=cdata, weight=cweight,layout='NCHW', pad=(3, 3), num_filter=64, stride=(2, 2), no_bias=True, dilate=(1, 1), kernel=(7, 7), num_group=1)

print(test.infer_shape_partial())

I get

([(1, 0, 0, 0), ()], [(1, 64, 0, 0)], [])

so the shape of the weight changed to ().

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Code is well-documented:
For new C++ functions in header files, their functionalities and arguments are documented.
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Change to the way passes are done so that both forward and backward inference is performed every time - I'm not sure if this is necessary - @eric-haibin-lin, thoughts?
Change to the way shape inference works. Currently a shape contributes 1 to the num_unknown if it has at least 1 zero. After the change number of 0 elements is added to the num_unknown - that way shape inference pass does not end prematurely if only some of the elements of shape were deduced.

Comments

ankkhedia · 2019-02-14T19:03:33Z

@ptrendx Thanks for your contribution! Could you please look into CI failures?

@mxnet-label-bot add [pr-awaiting-review , python]

eric-haibin-lin

Could you add an unit test for this?

@junrushao1994 @reminisce FYI ndim() API is used here.

src/executor/infer_graph_attr_pass.cc

reminisce

Thanks for the fix. I am not sure about the last two params. They were addd by @eric-haibin-lin. We can add comments in later PRs. This PR is good to merge.

reminisce · 2019-02-24T07:01:05Z

src/executor/infer_graph_attr_pass.cc

+ * \param fdefault default function used for inference if the node does not
+ * provide its own implementation.
+ * \param scalars_only whether the inferred attribute may be only a scalar
+ * \param bwd_identity_assign TODO


Only equal to false in inferring storage types. It means whether the attributes of forward ndarray and backward ndarray are necessarily the same.

eric-haibin-lin · 2019-02-24T18:55:07Z

src/executor/infer_graph_attr_pass.cc

+ * provide its own implementation.
+ * \param scalars_only whether the inferred attribute may be only a scalar
+ * \param bwd_identity_assign TODO
+ * \param dispatch_mode_name TODO


Name of the dispatch mode attribute on the node. Used for storage type inference.

eric-haibin-lin · 2019-02-24T18:55:52Z

src/executor/infer_graph_attr_pass.cc

+ * \param scalars_only whether the inferred attribute may be only a scalar
+ * \param bwd_identity_assign TODO
+ * \param dispatch_mode_name TODO
+ * param default_mode_val TODO


Default value of the dispatch mode attribute on the node. Used for storage type inference.

ptrendx · 2019-03-01T16:46:01Z

@junrushao1994 I wanted to rebase this PR on top of the current master and I'm confused - in PR #14193 you made InferShapeAttr as a standalone function, but then in #14270 you changed InferShape to use generic InferAttr again. Was this intentional? Which one should I use then?

junrushao · 2019-03-01T17:17:08Z

@ptrendx Sorry it is a mistake. I am actually working on two projects, dynamic shape and numpy operators. In dynamic shape, I should make infer shape pass standalone, but in numpy operator everything is just fine. I think these two projects interfere a bit.

ptrendx · 2019-03-01T18:08:03Z

Ok, so I should move it back to InferShapeAttr and make my changes there then?

junrushao · 2019-03-01T18:10:05Z

@ptrendx Yes, please. I am so sorry for the inconvenience.

ptrendx · 2019-03-04T22:32:12Z

@junrushao1994 @reminisce I think this PR is good to go. Do you have any other comments?

junrushao

LGTM.

This PR introduces fnum_unknown to the shape inference pass, which counts the number of unknown dimensions in an NDArray, and leverages this function to do shape inference more correctly.

* Fix InferShape pass * nnvm::TShape -> mxnet::TShape in InferShapeAttr * More nnvm->mxnet namespace changes * And more nnvm -> mxnet * Retrigger CI

marcoabreu added pr-awaiting-review PR is waiting for code review Python labels Feb 14, 2019

eric-haibin-lin reviewed Feb 15, 2019

View reviewed changes

src/executor/infer_graph_attr_pass.cc Outdated Show resolved Hide resolved

ptrendx mentioned this pull request Feb 15, 2019

MXNet AMP (automatic mixed precision) #14173

Merged

16 tasks

szha requested a review from reminisce February 16, 2019 04:38

reminisce reviewed Feb 21, 2019

View reviewed changes

src/executor/infer_graph_attr_pass.cc Outdated Show resolved Hide resolved

src/executor/infer_graph_attr_pass.cc Outdated Show resolved Hide resolved

src/executor/infer_graph_attr_pass.cc Outdated Show resolved Hide resolved

reminisce approved these changes Feb 24, 2019

View reviewed changes

eric-haibin-lin reviewed Feb 24, 2019

View reviewed changes

Fix InferShape pass

29ed75d

ptrendx force-pushed the pr_fix_shape_inference branch from 6aec7b6 to 29ed75d Compare March 1, 2019 18:31

ptrendx added 4 commits March 1, 2019 13:10

nnvm::TShape -> mxnet::TShape in InferShapeAttr

f8bc87a

More nnvm->mxnet namespace changes

9406372

And more nnvm -> mxnet

595ff44

Retrigger CI

0da7e54

junrushao approved these changes Mar 4, 2019

View reviewed changes

eric-haibin-lin merged commit 427b6d4 into apache:master Mar 5, 2019

vdantu pushed a commit to vdantu/incubator-mxnet that referenced this pull request Mar 31, 2019

Fix shape inference pass (apache#14153)

8482b75

* Fix InferShape pass * nnvm::TShape -> mxnet::TShape in InferShapeAttr * More nnvm->mxnet namespace changes * And more nnvm -> mxnet * Retrigger CI

nswamy pushed a commit that referenced this pull request Apr 5, 2019

Fix shape inference pass (#14153)

c35a82c

* Fix InferShape pass * nnvm::TShape -> mxnet::TShape in InferShapeAttr * More nnvm->mxnet namespace changes * And more nnvm -> mxnet * Retrigger CI

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix shape inference pass #14153

Fix shape inference pass #14153

ptrendx commented Feb 14, 2019 •

edited

Loading

ankkhedia commented Feb 14, 2019

eric-haibin-lin left a comment

reminisce left a comment

reminisce Feb 24, 2019

eric-haibin-lin Feb 24, 2019

eric-haibin-lin Feb 24, 2019

ptrendx commented Mar 1, 2019

junrushao commented Mar 1, 2019

ptrendx commented Mar 1, 2019

junrushao commented Mar 1, 2019

ptrendx commented Mar 4, 2019

junrushao left a comment •

edited

Loading

Fix shape inference pass #14153

Fix shape inference pass #14153

Conversation

ptrendx commented Feb 14, 2019 • edited Loading

Description

Checklist

Essentials

Changes

Comments

ankkhedia commented Feb 14, 2019

eric-haibin-lin left a comment

Choose a reason for hiding this comment

reminisce left a comment

Choose a reason for hiding this comment

reminisce Feb 24, 2019

Choose a reason for hiding this comment

eric-haibin-lin Feb 24, 2019

Choose a reason for hiding this comment

eric-haibin-lin Feb 24, 2019

Choose a reason for hiding this comment

ptrendx commented Mar 1, 2019

junrushao commented Mar 1, 2019

ptrendx commented Mar 1, 2019

junrushao commented Mar 1, 2019

ptrendx commented Mar 4, 2019

junrushao left a comment • edited Loading

Choose a reason for hiding this comment

ptrendx commented Feb 14, 2019 •

edited

Loading

junrushao left a comment •

edited

Loading