Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Fix deconvolution / PR 13421 #13433

Merged
merged 8 commits into from
Nov 30, 2018
Merged

Fix deconvolution / PR 13421 #13433

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b713021
add test case
azai91 Nov 28, 2018
c742913
revert refactor
azai91 Nov 28, 2018
6eb5b06
use with seed decorator
azai91 Nov 28, 2018
1f32ac1
retrigger
azai91 Nov 28, 2018
0d4160c
remove seed
azai91 Nov 28, 2018
34c8ef7
remove iteration
azai91 Nov 28, 2018
e310258
remove old test
azai91 Nov 29, 2018
d0c6d13
update deconvolution test to have filter length that triggers mkldnn …
azai91 Nov 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/operator/nn/mkldnn/mkldnn_deconvolution.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,14 @@ void MKLDNNDeconvForward::SetDataHandle(const DeconvolutionParam& param,
// For inference, we want to reorder the weight array so we don't need to
// reorder data every time.
if (weight.IsDefaultData()) {
weight_mem = GetWeights(weight, fwd_pd.weights_primitive_desc(), param.num_group);
// We also need to modify the layout on the original weight array. The
// data conversion happens after the weight array is used.
const_cast<NDArray&>(weight).MKLDNNDataReorderAsync(fwd_pd.weights_primitive_desc());
} else {
weight_mem = weight.GetMKLDNNData();
CHECK(weight_mem->get_primitive_desc() == fwd_pd.weights_primitive_desc());
}
weight_mem = weight.GetMKLDNNData();
CHECK(weight_mem->get_primitive_desc() == fwd_pd.weights_primitive_desc());
}
auto out_mem = CreateMKLDNNMem(out_data[deconv::kOut],
fwd_pd.diff_src_primitive_desc(), req[deconv::kOut]);
Expand Down
18 changes: 18 additions & 0 deletions tests/python/mkl/test_mkldnn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,24 @@ def softmax_forward(input_data, true_output):
softmax_forward(mx.nd.array([[[[-3.4e38,-3.4e38]]]]), np.array([1.0,1.0]))
softmax_forward(mx.nd.array([[[[3.4e38,3.4e38]]]]), np.array([1.0,1.0]))

@with_seed(12345)
Copy link
Member

Choose a reason for hiding this comment

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

No need to use a fixed seed here.

Copy link
Member

Choose a reason for hiding this comment

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

@TaoLv do you mean tests/python/unittest/test_operator.py. The tests in tests/python/gpu/test_operator_gpu.py are only meant to run with gpu context.
I see that there are tests in tests/python/unittest/test_operator.py for deconv but they are also skipped because of flakiness. Doesnt seem like we have any coverage on the Deconvolution operator for CPU

Copy link
Member

Choose a reason for hiding this comment

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

nvm. forgot about the check_consistency tests.

def test_deconvolution_inference():
num_filter = 256
num_group = 1
kernel = (3, 3)
pad = (1, 1)
shape = (1, 256, 200, 233)
x = mx.sym.Variable('x')
w = mx.sym.Variable('w')
y = mx.sym.Deconvolution(data=x, weight=w, num_filter=num_filter, num_group=num_group, kernel=kernel, no_bias=True, pad=pad)
exe = y.simple_bind(ctx=mx.cpu(), x=shape, grad_req='null')
exe.arg_arrays[0][:] = np.random.normal(size=exe.arg_arrays[0].shape)
exe.arg_arrays[1][:] = np.random.normal(size=exe.arg_arrays[1].shape)
for i in range(10):
Copy link
Member

Choose a reason for hiding this comment

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

nit: do we need this 10 times.

exe.forward(is_train=False)
o = exe.outputs[0]
t = o.asnumpy()
Copy link
Member

Choose a reason for hiding this comment

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

can we just do exe.outputs[0].wait_to_read()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right. this is only dependent on the shape and not values.


@with_seed()
def test_non_mkldnn_fcomputeex():
# test special case where MKLDNN formatted NDArray feeds into non-mkldnn fcomputeex operator
Expand Down