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

[1.4.x] Adds additional build envs #14920

Merged
merged 3 commits into from
May 9, 2019
Merged

[1.4.x] Adds additional build envs #14920

merged 3 commits into from
May 9, 2019

Conversation

perdasilva
Copy link
Contributor

@perdasilva perdasilva commented May 8, 2019 •
edited
Loading

Description

Takes elements of #14909 to add additional build environments for other CUDA versions for developer convenience and release automation. A follow-up PR should bump CI to CUDA v10 for this branch.

It also adds a build environment for compiling the statically linked libmxnet that forms the basis for the pip and maven releases.

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

@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
@perdasilva perdasilva changed the title [1.4.x] Update CI to use CUDA 10 and adds additional build envs [1.4.x] Adds additional build envs May 9, 2019
@perdasilva perdasilva changed the title [1.4.x] Adds additional build envs [WIP][1.4.x] Adds additional build envs May 9, 2019
@perdasilva
Copy link
Contributor Author

perdasilva commented May 9, 2019 •
edited
Loading

@lanking520 I have to pull in the statically linked binary build environment into this PR for the release automation. Could we do this now, and revert it once you have time for your more comprehensive PR? I'm happy to help you with that, but I would need this as a stop-gap solution in the meantime.

If this is ok with you, could you please merge?

@perdasilva perdasilva changed the title [WIP][1.4.x] Adds additional build envs [1.4.x] Adds additional build envs May 9, 2019
@lanking520
Copy link
Member

@zachgk Could you please review

lanking520
lanking520 reviewed May 9, 2019
View reviewed changes
Jenkinsfile
@@ -371,7 +371,7 @@ core_logic: {
ws('workspace/build-cmake-gpu') {
timeout(time: max_time, unit: 'MINUTES') {
utils.init_git()
utils.docker_run('ubuntu_gpu', 'build_ubuntu_gpu_cmake', false)
utils.docker_run('ubuntu_gpu_cu91', 'build_ubuntu_gpu_cmake', false)
Copy link
Member

Choose a reason for hiding this comment

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

Why do CU91 here? Scala do CU92 as a default

Copy link
Contributor Author

@perdasilva perdasilva May 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately, until very recently (#14513), CU91 was the version CI used (with the exception of the Scala stuff you guys added). I'm sticking with CU91 because that's what ubuntu_gpu is in this branch (see here).

CI in this branch definitely deserves an update. It should be brought to CUDA 10 (and once stu1130 is finished getting cudnn 7.5. working, CUDA 10.1). But I think this requires more time than I have atm.

zachgk
zachgk approved these changes May 9, 2019
View reviewed changes
Copy link
Contributor

@zachgk zachgk left a comment

Choose a reason for hiding this comment

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

This looks fine to me

@perdasilva
Copy link
Contributor Author

@zachgk thanks for looking into this. I've realized my description wasn't complete, and I've updated it. I hope you still agree with your assessment ^^

@zachgk zachgk merged commit b9ea0fa into apache:v1.4.x May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lanking520 lanking520 lanking520 left review comments

@zachgk zachgk zachgk approved these changes

@marcoabreu marcoabreu Awaiting requested review from marcoabreu marcoabreu is a code owner

Assignees
No one assigned
Labels
CI pr-awaiting-review PR is waiting for code review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@perdasilva @vandanavk @lanking520 @zachgk @marcoabreu