fix test_depthwise_convoltuion for occasional CI failures

[mseth10]

Description

This PR aims to fix #12203 . The test_depthwise convolution fails occasionally on CI for GPU context. The problem occurs as the test tries to match GPU output of convolution on an image with CPU output of convolution on sliced images concatenated later. The error tolerance values are sometimes exceeded. When the comparison instead is made between GPU output of convolution on an image with GPU output of convolution on sliced images concatenated later, it passes the test.

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)
• Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
• Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
• Code is well-documented:
• For user-facing API changes, API doc string has been updated.
• For new C++ functions in header files, their functionalities and arguments are documented.
• For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
• Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
• To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

• Second context changed to match first one. This make comparison for different implementations on same contexts

Comments

• If this change is a backward incompatible change, why must this change be made.
• Interesting edge cases to note here

[apeforest]

Can we also compare between cpu and cpu as context?

[apeforest]

Could you also create an issue of descprepancy between cpu and gpu implementation with the input to replay? Thanks!

[mseth10]

@apeforest We already do that. We actually compare between default contexts. So when default context is cpu, we compare between cpu and cpu.

[apeforest]

LGTM. @mseth10 Thanks for your thorough analysis!

[Vikas-kum]

Good catch!

I would also recommend that you should put if there is a input you tested this on. Overall this looks good to me.

On a side note, you should also investigate if there is a way to put the test for non-cudnn environment. I am not sure if we do this in mxnet CI, in case we do, the test should be part of that suite which runs test cases for non-cudnn environment.

[mseth10]

@Vikas89 I checked. We do have GPU no cuDNN environment running on our CI. I will make sure this test is running on that.
I will also add the test input for which this test was failing.

[lebeg]

Unfortunately, the same test is failing in this PR:

======================================================================
ERROR: test_operator.test_depthwise_convolution
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/work/mxnet/tests/python/unittest/common.py", line 173, in test_new
    orig_test(*args, **kwargs)
  File "/work/mxnet/tests/python/unittest/test_operator.py", line 1676, in test_depthwise_convolution
    np.testing.assert_allclose(arr1.asnumpy(), arr2.asnumpy(), rtol=1e-3, atol=1e-3)
  File "/work/mxnet/python/mxnet/ndarray/ndarray.py", line 1995, in asnumpy
    ctypes.c_size_t(data.size)))
  File "/work/mxnet/python/mxnet/base.py", line 252, in check_call
    raise MXNetError(py_str(_LIB.MXGetLastError()))
MXNetError: [01:40:38] src/operator/nn/mkldnn/mkldnn_base.cc:576: Check failed: similar 

Not sure whether it's the same error though.

[mseth10]

@lebeg It's not the same error - the current failure is on unix-cpu whereas earlier it was on unix-gpu. Also, I verified this failure is not due to the fix, it is because of some other code change that happened while this test was inactive.

[mseth10]

@Vikas89 This test already runs on GPU no cuDNN environment. So we are good there.

[lupesko]

Nice.
However, since the test is still failing (albeit due to a different root cause), we should probably keep skipping this test.

[mseth10]

Filed another issue #14052 for MKLDNN bug that causes CI failure when test is enabled. Keeping the test disabled as of now.

[vandanavk]

@mxnet-label-bot add [pr-awaiting-merge, Flaky, Test]

[yuxihu]

Good catch! LGTM.