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

[CI] Test CMake builds instead of Makefile builds #17645

Merged
merged 1 commit into from Feb 28, 2020
Merged

[CI] Test CMake builds instead of Makefile builds #17645

merged 1 commit into from Feb 28, 2020

Conversation

leezu
Copy link
Contributor

@leezu leezu commented Feb 21, 2020 •
edited

Description

Use cmake build for most CI builds in preparation of dropping Makefile build

Fixes #17548

Comments

The following Makefile based builds are preserved

  1. staticbuild scripts. Changes to CD pipeline will happen in Switch CD to cmake build #17646
  2. Docs builds. Language binding specific build logic requires further changes
  3. Jetson build. Jetpack 3.3 toolchain based on Cuda 9.0 causes 'Internal
    Compiler Error (codegen): "there was an error in verifying the lgenfe
    output!"' errors with cmake. This seems to be a known issue in Cuda 9.0 and
    we need to update Jetpack toolchain to work around it.
  4. MKL builds. Waiting for fix of OpenMP Error #17641

Improvements to CMake build

  • Enable -Werror for RelWithDebugInfo build in analogy to "make DEV=1" build
  • Add USE_LIBJPEG_TURBO to CMake build
  • Improve finding Python 3 executable

Changes to CI setup

  • Install protobuf in ubuntu_core.sh and centos_core.sh
  • Install up-to-date CMake on Centos 7
  • Don't use RelWithDebInfo on Android builds, as gcc 4.9 throws
    -Wdelete-non-virtual-dtor

Code changes

  • Locally disable warnings introduced by GCC7

@leezu leezu force-pushed the citestcmake branch 6 times, most recently from aebaae0 to 46adbfb Compare February 21, 2020 06:36
@leezu
Copy link
Contributor Author

leezu commented Feb 21, 2020 •
edited

@apeforest I agree the TVM Op testing can be improved. I'll first attempt to make this PR pass CI, without changing any of the build configurations tested (but only switch the build system).
Once CI passes, we can consider merging and have a separate PR for changing configurations or iterate on this PR.

@leezu leezu force-pushed the citestcmake branch 9 times, most recently from 527f7c8 to 5e70fd7 Compare February 21, 2020 19:00
@apeforest
Copy link
Contributor

Could you first test this in the mxnet-ci-dev environment? If there is anything wrong, we don't want it to break the CI production right?

@leezu leezu force-pushed the citestcmake branch 3 times, most recently from 7984f2c to 362813c Compare February 27, 2020 06:53
@leezu leezu requested a review from nswamy as a code owner February 27, 2020 06:53
@leezu leezu force-pushed the citestcmake branch 2 times, most recently from a4615fb to abede68 Compare February 27, 2020 23:05
@leezu
CI: Switch to cmake builds for majority of tests
eff3085 
In particular the following Makefile based builds are preserved
1) staticbuild scripts
2) Docs builds. Language binding specific build logic requires further changes
3) Jetson build. Jetpack 3.3 toolchain based on Cuda 9.0 causes 'Internal
   Compiler Error (codegen): "there was an error in verifying the lgenfe
   output!"' errors with cmake. This seems to be a known issue in Cuda 9.0 and
   we need to update Jetpack toolchain to work around it.

All Makefile based builds are marked with a "Makefile" postfix in the title.

Improvements to CMake build
- Enable -Werror for RelWithDebugInfo build in analogy to "make DEV=1" build
- Add USE_LIBJPEG_TURBO to CMake build
- Improve finding Python 3 executable

Changes to CI setup
- Install protobuf in ubuntu_core.sh and centos_core.sh
- Install up-to-date CMake on Centos 7
- Don't use RelWithDebInfo on Android builds, as gcc 4.9 throws
  -Wdelete-non-virtual-dtor

Code changes
- Locally disable warnings introduced by GCC7
@leezu leezu mentioned this pull request Feb 28, 2020
Open
@leezu leezu merged commit 319e6c1 into apache:master Feb 28, 2020
@leezu leezu deleted the citestcmake branch February 28, 2020 19:59
@leezu leezu mentioned this pull request Feb 28, 2020
Merged
leezu added a commit to leezu/mxnet that referenced this pull request Mar 5, 2020
@leezu
Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy
fd15c1e 
Fixes a regression in CD introduced by
apache#17645
@leezu leezu mentioned this pull request Mar 5, 2020
Merged
leezu added a commit that referenced this pull request Mar 5, 2020
@leezu
Fix CD (#17776)
7c61cda 
* Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy

Fixes a regression in CD introduced by
#17645

* Fix whitespace

* Add NATIVE_ADDITIONAL.md

Fixes a regression in CD introduced by
#16899

* Update other $TYPE_ADDITIONAL.MD

* Fix cd/python/docker

Fixes regression introduced by
#15990
MoisesHer pushed a commit to MoisesHer/incubator-mxnet that referenced this pull request Apr 10, 2020
@leezu @MoisesHer
CI: Switch to cmake builds for majority of tests (apache#17645)
48d9d0c 
The following Makefile based builds are preserved
1) staticbuild scripts
2) Docs builds. Language binding specific build logic requires further changes
3) Jetson build. Jetpack 3.3 toolchain based on Cuda 9.0 causes 'Internal
   Compiler Error (codegen): "there was an error in verifying the lgenfe
   output!"' errors with cmake. This seems to be a known issue in Cuda 9.0 and
   we need to update Jetpack toolchain to work around it.
4) MKL builds. Waiting for fix of apache#17641

All Makefile based builds are marked with a "Makefile" postfix in the title.

Improvements to CMake build
- Enable -Werror for RelWithDebugInfo build in analogy to "make DEV=1" build
- Add USE_LIBJPEG_TURBO to CMake build
- Improve finding Python 3 executable

Changes to CI setup
- Install protobuf and zmq where missing
- Install up-to-date CMake on Centos 7
- Don't use RelWithDebInfo on Android builds, as gcc 4.9 throws
  -Wdelete-non-virtual-dtor

Code changes
- Disable warnings introduced by GCC7 at via #pragma GCC diagnostic
MoisesHer pushed a commit to MoisesHer/incubator-mxnet that referenced this pull request Apr 10, 2020
@leezu @MoisesHer
Fix CD (apache#17776)
6d44e10 
* Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy

Fixes a regression in CD introduced by
apache#17645

* Fix whitespace

* Add NATIVE_ADDITIONAL.md

Fixes a regression in CD introduced by
apache#16899

* Update other $TYPE_ADDITIONAL.MD

* Fix cd/python/docker

Fixes regression introduced by
apache#15990
@leezu leezu mentioned this pull request Apr 15, 2020
Closed
@TaoLv TaoLv mentioned this pull request Apr 15, 2020
Merged
7 tasks
anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 29, 2020
@leezu @anirudh2290
CI: Switch to cmake builds for majority of tests (apache#17645)
fa6e45e 
The following Makefile based builds are preserved
1) staticbuild scripts
2) Docs builds. Language binding specific build logic requires further changes
3) Jetson build. Jetpack 3.3 toolchain based on Cuda 9.0 causes 'Internal
   Compiler Error (codegen): "there was an error in verifying the lgenfe
   output!"' errors with cmake. This seems to be a known issue in Cuda 9.0 and
   we need to update Jetpack toolchain to work around it.
4) MKL builds. Waiting for fix of apache#17641

All Makefile based builds are marked with a "Makefile" postfix in the title.

Improvements to CMake build
- Enable -Werror for RelWithDebugInfo build in analogy to "make DEV=1" build
- Add USE_LIBJPEG_TURBO to CMake build
- Improve finding Python 3 executable

Changes to CI setup
- Install protobuf and zmq where missing
- Install up-to-date CMake on Centos 7
- Don't use RelWithDebInfo on Android builds, as gcc 4.9 throws
  -Wdelete-non-virtual-dtor

Code changes
- Disable warnings introduced by GCC7 at via #pragma GCC diagnostic
anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 29, 2020
@leezu @anirudh2290
Fix CD (apache#17776)
52eb206 
* Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy

Fixes a regression in CD introduced by
apache#17645

* Fix whitespace

* Add NATIVE_ADDITIONAL.md

Fixes a regression in CD introduced by
apache#16899

* Update other $TYPE_ADDITIONAL.MD

* Fix cd/python/docker

Fixes regression introduced by
apache#15990
mseth10 pushed a commit to mseth10/incubator-mxnet that referenced this pull request Jun 2, 2020
@leezu @mseth10
Fix CD (apache#17776)
b8bcb16 
* Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy

Fixes a regression in CD introduced by
apache#17645

* Fix whitespace

* Add NATIVE_ADDITIONAL.md

Fixes a regression in CD introduced by
apache#16899

* Update other $TYPE_ADDITIONAL.MD

* Fix cd/python/docker

Fixes regression introduced by
apache#15990
mseth10 pushed a commit to mseth10/incubator-mxnet that referenced this pull request Jun 2, 2020
@leezu @mseth10
Fix CD (apache#17776)
e4d1173 
* Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy

Fixes a regression in CD introduced by
apache#17645

* Fix whitespace

* Add NATIVE_ADDITIONAL.md

Fixes a regression in CD introduced by
apache#16899

* Update other $TYPE_ADDITIONAL.MD

* Fix cd/python/docker

Fixes regression introduced by
apache#15990
leezu added a commit that referenced this pull request Jun 9, 2020
@mseth10 @leezu @szha
[v1.7] Cherry-pick #17776 #17681 (#18464)
a03b1fc 
* Fix CD (#17776)

* Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy

Fixes a regression in CD introduced by
#17645

* Fix whitespace

* Add NATIVE_ADDITIONAL.md

Fixes a regression in CD introduced by
#16899

* Update other $TYPE_ADDITIONAL.MD

* Fix cd/python/docker

Fixes regression introduced by
#15990

* [CD] update pypi description, setup.py (#17681)

* update pypi description, setup.py, use manylinux2010, use unified dist link for nightly

* Use manylinux2014

Co-authored-by: Leonard Lausen <leonard@lausen.nl>

* reverting .so path as per MAKE flow

Co-authored-by: Leonard Lausen <lausen@amazon.com>
Co-authored-by: Sheng Zha <szha@users.noreply.github.com>
Co-authored-by: Leonard Lausen <leonard@lausen.nl>
leezu added a commit that referenced this pull request Jun 15, 2020
@mseth10 @leezu @szha
[v1.x] Cherry-pick #17776 #17681 (#18465)
3b2c9ad 
* Fix CD (#17776)

* Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy

Fixes a regression in CD introduced by
#17645

* Fix whitespace

* Add NATIVE_ADDITIONAL.md

Fixes a regression in CD introduced by
#16899

* Update other $TYPE_ADDITIONAL.MD

* Fix cd/python/docker

Fixes regression introduced by
#15990

* [CD] update pypi description, setup.py (#17681)

* update pypi description, setup.py, use manylinux2010, use unified dist link for nightly

* Use manylinux2014

Co-authored-by: Leonard Lausen <leonard@lausen.nl>

* reverting .so path as per MAKE flow

Co-authored-by: Leonard Lausen <lausen@amazon.com>
Co-authored-by: Sheng Zha <szha@users.noreply.github.com>
Co-authored-by: Leonard Lausen <leonard@lausen.nl>
@sxjscience
Copy link
Member

@ciyongch Would this go to v1.7? I come up to this commit when I'm cherry-picking the related commits one by one

@sxjscience sxjscience mentioned this pull request Jul 1, 2020
Closed
7 tasks
@ciyongch
Copy link
Contributor

ciyongch commented Jul 3, 2020

Hi @sxjscience , sorry for overlooking your message. I don't see there's a strong requirement for this and likely not to include this patch in v1.7.

ChaiBapchya pushed a commit to ChaiBapchya/mxnet that referenced this pull request Aug 15, 2020
@mseth10 @leezu
@szha @ChaiBapchya
[v1.x] Cherry-pick apache#17776 apache#17681 (apache#18465)
afa1398 
* Fix CD (apache#17776)

* Fix cd/mxnet_lib/dynamic/Jenkins_pipeline.groovy

Fixes a regression in CD introduced by
apache#17645

* Fix whitespace

* Add NATIVE_ADDITIONAL.md

Fixes a regression in CD introduced by
apache#16899

* Update other $TYPE_ADDITIONAL.MD

* Fix cd/python/docker

Fixes regression introduced by
apache#15990

* [CD] update pypi description, setup.py (apache#17681)

* update pypi description, setup.py, use manylinux2010, use unified dist link for nightly

* Use manylinux2014

Co-authored-by: Leonard Lausen <leonard@lausen.nl>

* reverting .so path as per MAKE flow

Co-authored-by: Leonard Lausen <lausen@amazon.com>
Co-authored-by: Sheng Zha <szha@users.noreply.github.com>
Co-authored-by: Leonard Lausen <leonard@lausen.nl>
@adamjstewart adamjstewart mentioned this pull request Feb 2, 2021
Merged
7 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@apeforest apeforest apeforest approved these changes

@TaoLv TaoLv TaoLv left review comments

@cjolivier01 cjolivier01 cjolivier01 approved these changes

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

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

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

@anirudh2290 anirudh2290 Awaiting requested review from anirudh2290

@eric-haibin-lin eric-haibin-lin Awaiting requested review from eric-haibin-lin

@nswamy nswamy Awaiting requested review from nswamy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CI doesn't test cmake build properly
6 participants
@leezu @apeforest @sxjscience @ciyongch @cjolivier01 @TaoLv