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

[MXNET-11241] Avoid use of troublesome cudnnFind() results when grad_req='add' #11338

Merged
merged 4 commits into from
Jul 30, 2018

Conversation

DickJC123
Copy link
Contributor

Description

The problem of issue #11241 arises because cudnnFind() measures convolution algorithm runtimes with an assumed "output blending parameter" beta of 0. However, algorithms may have specialized kernels for the beta==0 case, different and faster than the generalized beta kernels. Should the generalized kernels have issues with the problem size different than the beta==0 kernels, then the algos returned by cudnnFind() might fail when invoked with beta==1 (as it is when the convolution op grad_req='add' argument is present).

The demonstrated problem area involves a large 'c' value of 64K, where for the backprop-to-filter kernel only algo 1 handles the beta==1 case. CudnnFind() was shown to occasionally return algos 0 or 3 as fastest, and both of these return error 8 "execution failed" when run.

The fix is based on the observation that cudnnGet() returns algo 1 for the backprop-to-filter kernel for the troublesome problem sizes. Thus, the fix is to avoid cudnnFind() when grad_req='add' and force use of cudnnGet() instead. The fix maintains the effectiveness of the caching of algo lookups and convolution op instances, so neither cudnnFind() nor cudnnGet() is called repeatedly. Deconvolution was similarly updated with this fix.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • [X ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • [X ] Changes are complete (i.e. I finished coding on this PR) [1st commit includes test, 2nd the fix]
  • [X ] 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)
  • [X ] 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
  • [X ] 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

@DickJC123
Copy link
Contributor Author

My second commit did not pass because the CI exposed a flaw in the logic of the fix: while cudnnGet() returned the required algo 1 on Volta, it did not for the Maxwell GPUs that were part of the CI cluster. A third commit narrows the scope of the fix to the observed failure scenarios (grad_req='add' and c>=64K). The fix is to force the problematic backprop-to-filter kernel to use algo 1 in this case. The CI failed on this commit as well, but for seemingly unrelated reasons. Will try an empty commit.

@eric-haibin-lin
Copy link
Member

eric-haibin-lin commented Jul 10, 2018

do you mind rebasing on mxnet master and see if CI works?

@DickJC123 DickJC123 force-pushed the fix_for_cudnnfind_issue branch from 4070348 to 7dfe9e8 Compare July 16, 2018 21:37
@DickJC123 DickJC123 requested a review from anirudh2290 as a code owner July 16, 2018 21:37
@eric-haibin-lin eric-haibin-lin merged commit 024b5a9 into apache:master Jul 30, 2018
aaronmarkham added a commit to aaronmarkham/incubator-mxnet that referenced this pull request Aug 7, 2018
* adding param for list of tags to display on website

* using new website display argument for artifact placement in version folder

* adding display logic

* remove restricted setting for testing

* update usage instructions

* reverted Jenkinsfile to use restricted nodes

[MXAPPS-581] Fixes for broken Straight Dope tests. (apache#11923)

* Update relative paths pointing to the data directory to point to the
  correct place in the testing temporary folder.

* Enable the notebooks that were previously broken because of relative
  file paths not pointing to the correct place.

* Move some notebooks we do not plan to test to the whitelist. These
  notebooks are not published in the Straight Dope book.

* Clean-up: Convert print statements to info/warn/error logging
  statements. Add some logging statements for better status.

Disable flaky test: test_spatial_transformer_with_type (apache#11930)

apache#11839

Add linux and macos MKLDNN Building Instruction (apache#11049)

* add linux and macos doc

* update doc

* Update MKL_README.md

* Update MKL_README.md

Add convolution code to verify mkldnn backend

* add homebrew link

* rename to MKLDNN_README

* add mkl verify

* trigger

* trigger

* set mac complier to gcc47

* add VS2017 support experimentally

* improve quality

* improve quality

* modify mac build instruction since prepare_mkldnn.sh has been rm

* trigger

* add some improvement

[MXNET-531] Add download util (apache#11866)

* add changes to example

* place the file to the util

* add retry scheme

* fix the retry logic

* change the DownloadUtil to Util

* Trigger the CI

[MXNET-11241] Avoid use of troublesome cudnnFind() results when grad_req='add' (apache#11338)

* Add tests that fail due to issue 11241

* Fix apache#11241 Conv1D throws CUDNN_STATUS_EXECUTION_FAILED

* Force algo 1 when grad_req==add with large c.  Expand tests.

* Shorten test runtimes.

Improving documentation and error messages for Async distributed training with Gluon (apache#11910)

* Add description about update on kvstore

* add async check for gluon

* only raise error if user set update_on_kvstore

* fix condition

* add async nightly test

* fix case when no kvstore

* add example for trainer creation in doc

[MXNET-641] fix R windows install docs (apache#11805)

* fix R windows install docs

* addressed PR comments

* PR comments

* PR comments

* fixed line wrappings

* fixed line wrappings

a hot fix for mkldnn link (apache#11939)

re-enabling randomized test_l2_normalization (apache#11900)

[MXNET-651] MXNet Model Backwards Compatibility Checker (apache#11626)

* Added MNIST-MLP-Module-API models to check model save and load_checkpoint methods

* Added LENET with Conv2D operator training file

* Added LENET with Conv2d operator inference file

* Added LanguageModelling with RNN training file

* Added LamguageModelling with RNN inference file

* Added hybridized LENET Gluon Model training file

* Added hybridized LENET gluon model inference file

* Added license headers

* Refactored the model and inference files and extracted out duplicate code in a common file

* Added runtime function for executing the MBCC files

* Added JenkinsFile for MBCC to be run as a nightly job

* Added boto3 install for s3 uploads

* Added README for MBCC

* Added license header

* Added more common functions from lm_rnn_gluon_train and inference files into common.py to clean up code

* Added scripts for training models on older versions of MXNet

* Added check for preventing inference script from crashing in case no trained models are found

* Fixed indentation issue

* Replaced Penn Tree Bank Dataset with Sherlock Holmes Dataset

* Fixed indentation issue

* Removed training in models and added smaller models. Now we are simply checking a forward pass in the model with dummy data.

* Updated README

* Fixed indentation error

* Fixed indentation error

* Removed code duplication in the training file

* Added comments for runtime_functions script for training files

* Merged S3 Buckets for storing data and models into one

* Automated the process to fetch MXNet versions from git tags

* Added defensive checks for the case where the data might not be found

* Fixed issue where we were performing inference on state model files

* Replaced print statements with logging ones

* Removed boto install statements and move them into ubuntu_python docker

* Separated training and uploading of models into separate files so that training runs in Docker and upload runs outside Docker

* Fixed pylint warnings

* Updated comments and README

* Removed the venv for training process

* Fixed indentation in the MBCC Jenkins file and also separated out training and inference into two separate stages

* Fixed indendation

* Fixed erroneous single quote

* Added --user flag to check for Jenkins error

* Removed unused methods

* Added force flag in the pip command to install mxnet

* Removed the force-re-install flag

* Changed exit 1 to exit 0

* Added quotes around the shell command

* added packlibs and unpack libs for MXNet builds

* Changed PythonPath from relative to absolute

* Created dedicated bucket with correct permission

* Fix for python path in training

* Changed bucket name to CI bucket

* Added set -ex to the upload shell script

* Now raising an exception if no models are found in the S3 bucket

* Added regex to train models script

* Added check for performing inference only on models trained on same major versions

* Added set -ex flags to shell scripts

* Added multi-version regex checks in training

* Fixed typo in regex

* Now we will train models for all the minor versions for a given major version by traversing the tags

* Added check for validating current_version

[MXNET-531] NeuralStyle Example for Scala (apache#11621)

* add initial neuralstyle and test coverage

* Add two more test and README

* kill comments

* patch on memory leaks fix

* fix formatting issues

* remove redundant files

* disable the Gan example for now

* add ignore method

* add new download scheme to match the changes
XinYao1994 pushed a commit to XinYao1994/incubator-mxnet that referenced this pull request Aug 29, 2018
…req='add' (apache#11338)

* Add tests that fail due to issue 11241

* Fix apache#11241 Conv1D throws CUDNN_STATUS_EXECUTION_FAILED

* Force algo 1 when grad_req==add with large c.  Expand tests.

* Shorten test runtimes.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants