-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fallback to dense version for grad(reshape), grad(expand_dims) #13599
Conversation
.set_attr<FResourceRequest>("FResourceRequest", [](const NodeAttrs& n) { | ||
return std::vector<ResourceRequest>{ResourceRequest::kTempSpace}; | ||
}) | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does mkldnn support reshape?
if you want to optimize for mkldnn, you should add FComputeEx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, #12980 is enabled the FW of MKL-DNN supported reshape but BW is still WIP @huangzhiyuan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I guess we need to remove this for now.
Otherwise, it looks good to me. BTW, I don't think it has anything to do with sparse. |
ccc66e1
to
986803c
Compare
[](const NodeAttrs& attrs){ | ||
return std::vector<std::pair<int, int> >{{0, 0}}; | ||
}) | ||
.set_attr<FInferStorageType>("FInferStorageType", ElemwiseStorageType<1, 1, false, false, false>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an op only supports dense tensors and FCompute, FInferStorageType is not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated, thanks!
986803c
to
04587b5
Compare
…e#13599) * fallback to dense version for grad(reshape), grad(expand_dims) * add _backward_reshape gpu version * reshape test case comments * fix gpu test * remove mkldnn support for _backward_reshape
…e#13599) * fallback to dense version for grad(reshape), grad(expand_dims) * add _backward_reshape gpu version * reshape test case comments * fix gpu test * remove mkldnn support for _backward_reshape
Description
Current
reshape
uses_backward_copy
to do gradient calculation.But
_backward_copy
sparse calculation requires same shapes of input and output - which is fair since sparsity is bind to # of rows and cols.And though
reshape
itself does not have sparse version, we can easily construct a network, in which its backward input and output are sparse - see test case.Thus we need to provide a fallback version for
reshape
backward computation, so asexpand_dims
.@eric-haibin-lin @zheng-da
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.