Fix build issue with USE_CUDNN=0 #11470

anirudh2290 · 2018-06-28T20:33:39Z

Description

This was found during the flaky test investigation #10973 . I found that the windows code path is different from linux, so it was testing the MXNet conv and deconv op which was not being tested for linux. Building with USE_CUDNN=0 and USE_CUDA=1 was failing and I have added the fix here. I have also added the build and test as a stage to jenkins. Not sure if there was a reason to not check this in CI (configuration not supported by some ops etc.)

@marcoabreu

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
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

Feature1, tests, (and when applicable, API doc)
Feature2, tests, (and when applicable, API doc)

Comments

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

marcoabreu · 2018-06-28T20:36:23Z

Jenkinsfile

+        }
+      }
+    },
+    'Python3: MKLDNN-GPU-NOCUDNN': {


Is it needed to test python2 and python3 or is one enough?

one is enough , i will remove python2.

marcoabreu · 2018-06-28T20:37:25Z

Jenkinsfile

+         ws('workspace/build-mkldnn-gpu-nocudnn') {
+            timeout(time: max_time, unit: 'MINUTES') {
+               init_git()
+               docker_run('ubuntu_build_cuda', 'build_ubuntu_gpu_mkldnn_cuda', false)


supposed to be build_ubuntu_gpu_mkldnn_nocudnn?

good catch! will fix.

marcoabreu · 2018-06-28T21:01:31Z

ci/docker/runtime_functions.sh

+
+    make  \
+        DEV=1                         \
+        USE_CPP_PACKAGE=1             \


No need for the cpp package here since we don't run the cpp tests

not sure about this . shouldnt we test if it builds fine with CUDNN with CPP_PACKAGE just in case cpp package depends on the flag to use cudnn specific functionality (unlikely) ?

The CPP package just generates wrappers around the C-API and should not rely on backend-specific things like CUDA or CUDNN. Thus, I'd say that we should keep it separate and only build the CPP package if we're actually going to run it (which we do in another test)

okay, removed!

marcoabreu

Lgtm besides the cpp package

szha

CI failure seems to be caused by the assumption that GPU CI runs all have cudnn enabled. Could you propose a fix for that?

anirudh2290 · 2018-06-29T20:42:27Z

@szha Yep, planning to fix that.

anirudh2290 · 2018-07-05T21:56:38Z

the spatial transformer op seems to be a blocker with a bunch of issues when cudnn disabled: #11568. I am currently hiding the assertion error for this op and triggering ci again. Need to stop hiding the assertion error when the issues with the op are resolved: #11568.

anirudh2290 · 2018-07-06T19:00:17Z

The two issues : #11568 and #11588 summarize the ops that are not tested and errors hidden for cuDNN disabled. I think it is okay to ignore these ops testing for cudnn disabled for now. This PR should be good to go. Removing WIP. Please help review.

piiswrong · 2018-07-09T21:37:01Z

include/mxnet/c_api.h

+ * \param curr returns the current status.
+ * \return 0 when success, -1 when failure happens
+ */
+MXNET_DLL int MXCUDNNIsEnabled(bool* curr);


Please don't add CAPI for this.
Add a environment variable if you need it for testing

marcoabreu · 2018-07-11T18:17:20Z

We have to take care of conflicting changes when merging this PR and #11630.

anirudh2290 · 2018-07-11T21:18:12Z

I have resolved the conflicts and addressed the comments on #11630

marcoabreu

Lgtm, thanks a lot!

haojin2 · 2018-07-17T17:44:04Z

This should fix #11339

* Fix build issue with CUDNN=0 * Fix nocudnn func name * Remove python2 tests * Remove CPP package test * Check assert raises when cudnn disabled for op tests on gpu * Add line * Remove whitespace * add decorator for other ops * Add and remove assert * Fix op and common * Fix merge issue * Remove C API * Fix * Fix lint * Add init git * Rename CUDNN_DISABLED env variable * Add a runtime function for nocudnn * Remove MXCudnnIsenabled * Add comment for disabled test * Add full link in comment

Fix build issue with CUDNN=0

6961a20

anirudh2290 requested a review from marcoabreu as a code owner June 28, 2018 20:33

marcoabreu reviewed Jun 28, 2018

View reviewed changes

anirudh2290 mentioned this pull request Jun 28, 2018

Flaky test_deconvolution #10973

Closed

anirudh2290 added 2 commits June 28, 2018 20:47

Fix nocudnn func name

7bdf6c7

Remove python2 tests

06815b4

eric-haibin-lin approved these changes Jun 28, 2018

View reviewed changes

marcoabreu reviewed Jun 28, 2018

View reviewed changes

marcoabreu approved these changes Jun 28, 2018

View reviewed changes

Remove CPP package test

def0842

marcoabreu approved these changes Jun 28, 2018

View reviewed changes

szha reviewed Jun 29, 2018

View reviewed changes

anirudh2290 added 2 commits July 2, 2018 18:51

Check assert raises when cudnn disabled for op tests on gpu

e7e6084

Merge with master

a488e43

anirudh2290 changed the title ~~Fix build issue with USE_CUDNN=0~~ [WIP] Fix build issue with USE_CUDNN=0 Jul 2, 2018

anirudh2290 added 6 commits July 2, 2018 19:04

Add line

0631682

Remove whitespace

2d11ade

add decorator for other ops

f338866

Fix tests

2c7453a

Add and remove assert

a5c2c79

Fix op and common

71d7d72

anirudh2290 mentioned this pull request Jul 5, 2018

Issues with spatial transformer op when cudnn disabled #11568

Closed

Fix merge issue

65d01e0

anirudh2290 mentioned this pull request Jul 6, 2018

RNN when CUDNN is disabled #11588

Open

anirudh2290 changed the title ~~[WIP] Fix build issue with USE_CUDNN=0~~ Fix build issue with USE_CUDNN=0 Jul 6, 2018

piiswrong suggested changes Jul 9, 2018

View reviewed changes

Remove C API

4ce776c

anirudh2290 changed the title ~~Fix build issue with USE_CUDNN=0~~ [WIP] Fix build issue with USE_CUDNN=0 Jul 10, 2018

anirudh2290 added 2 commits July 10, 2018 06:50

Fix

556dee1

Fix lint

67d00a3

anirudh2290 mentioned this pull request Jul 10, 2018

Fix flaky test test_deconvolution #11630

Merged

anirudh2290 added 4 commits July 10, 2018 18:25

Add init git

266ba86

Rename CUDNN_DISABLED env variable

e837777

Add a runtime function for nocudnn

2f590ee

Resolve conflicts

a2fc5f5

marcoabreu mentioned this pull request Jul 11, 2018

Fix for #11339 #11652

Closed

5 tasks

anirudh2290 added 2 commits July 11, 2018 21:09

Remove MXCudnnIsenabled

749981d

Add comment for disabled test

9573913

haojin2 mentioned this pull request Jul 11, 2018

compiling error when USE_CUDA=1 and USE_CUDNN=0 with make command #11339

Closed

Add full link in comment

e8d87dd

marcoabreu approved these changes Jul 11, 2018

View reviewed changes

piiswrong approved these changes Jul 12, 2018

View reviewed changes

anirudh2290 changed the title ~~[WIP] Fix build issue with USE_CUDNN=0~~ Fix build issue with USE_CUDNN=0 Jul 12, 2018

anirudh2290 merged commit 5b63144 into apache:master Jul 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix build issue with USE_CUDNN=0 #11470

Fix build issue with USE_CUDNN=0 #11470

anirudh2290 commented Jun 28, 2018

marcoabreu Jun 28, 2018

anirudh2290 Jun 28, 2018

marcoabreu Jun 28, 2018

anirudh2290 Jun 28, 2018

marcoabreu Jun 28, 2018

anirudh2290 Jun 28, 2018

marcoabreu Jun 28, 2018

anirudh2290 Jun 28, 2018

marcoabreu left a comment

szha left a comment

anirudh2290 commented Jun 29, 2018

anirudh2290 commented Jul 5, 2018

anirudh2290 commented Jul 6, 2018

piiswrong Jul 9, 2018

marcoabreu commented Jul 11, 2018

anirudh2290 commented Jul 11, 2018

marcoabreu left a comment

haojin2 commented Jul 17, 2018

Fix build issue with USE_CUDNN=0 #11470

Fix build issue with USE_CUDNN=0 #11470

Conversation

anirudh2290 commented Jun 28, 2018

Description

Checklist

Essentials

Changes

Comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoabreu left a comment

Choose a reason for hiding this comment

szha left a comment

Choose a reason for hiding this comment

anirudh2290 commented Jun 29, 2018

anirudh2290 commented Jul 5, 2018

anirudh2290 commented Jul 6, 2018

Choose a reason for hiding this comment

marcoabreu commented Jul 11, 2018

anirudh2290 commented Jul 11, 2018

marcoabreu left a comment

Choose a reason for hiding this comment

haojin2 commented Jul 17, 2018