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

Updates to cudnn package installation #14923

Merged
merged 2 commits into from
May 12, 2019

Conversation

perdasilva
Copy link
Contributor

@perdasilva perdasilva commented May 9, 2019

Description

Currently, the installation script for centos7 cudnn only installs a single version of cudnn independendly of what is set in the CUDNN_VERSION environment variable. This PR aims to fix it by making it more general by constructing the download url for the cudnn package from the CUDA_VERSION and CUDNN_VERSION environment variables.

Additionally, by having the CUDNN_VERSION defined at the start of the Dockerfile, the whole docker file will be rebuilt if there's a change. Therefore, moving it next to the call to the install cudnn script.

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

@perdasilva perdasilva force-pushed the cudnn_centos_install_fix branch 2 times, most recently from 55a882f to 5c0cf00 Compare May 9, 2019 08:03
@vandanavk
Copy link
Contributor

@mxnet-label-bot add [pr-awaiting-review, CI]

@marcoabreu marcoabreu added CI pr-awaiting-review PR is waiting for code review labels May 9, 2019
@@ -22,11 +22,38 @@

set -ex

if [ -z ${CUDA_VERSION} ]; then
echo "Error: CUDA_VERSION environment variable undefiend"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: undefined

fi

if [ -z ${CUDNN_VERSION} ]; then
echo "Error: CUDNN_VERSION environment variable undefiend"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eyes! It's fixed now.

@perdasilva perdasilva changed the title Generalizes centos7 cudnn download and install script Updates to cudnn package installation May 10, 2019
@perdasilva
Copy link
Contributor Author

@marcoabreu please review in light of our discussion

@wkcn wkcn merged commit 669ab2c into apache:master May 12, 2019
@wkcn
Copy link
Member

wkcn commented May 12, 2019

Merged. Thank you!

haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* Generalizes centos7 cudnn download and install script

* Updates setting of cudnn version to a position in the Dockerfile that will have the least impact on caching
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants