You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/02/21 02:12:41 UTC

[GitHub] [incubator-mxnet] leezu opened a new pull request #17645: [CI] Test CMake builds instead of Makefile builds

leezu opened a new pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645
 
 
   ## Description ##
   Use cmake build for all CI builds in preparation of dropping Makefile build
   
   Fixes https://github.com/apache/incubator-mxnet/issues/17548
   
   ## Comments ##
   - Enable WError for RelWithDebug build
   - Add USE_LIBJPEG_TURBO to CMake build

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu edited a comment on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu edited a comment on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-589528150
 
 
   @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.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384749586
 
 

 ##########
 File path: 3rdparty/mshadow/mshadow/base.h
 ##########
 @@ -839,7 +840,7 @@ MSHADOW_XINLINE bool MaxValue<bool>(void) {
 /*! \brief maximum value of uint32_t */
 template<>
 MSHADOW_XINLINE uint32_t MaxValue<uint32_t>(void) {
-  return -1;
+  return std::numeric_limits<uint32_t>::max();
 
 Review comment:
   Fixes a compiler warning treated as error due to `-WError` on CI.
   Essentially we're currently relying on unsigned integer overflow to obtain the max value. `std::numeric_limits<uint32_t>::max();` gives the max value directly

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421140
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -455,36 +437,40 @@ build_ubuntu_cpu() {
 
 build_ubuntu_cpu_openblas() {
     set -ex
-    export CC="gcc"
-    export CXX="g++"
-    build_ccache_wrappers
-    make \
-        DEV=1                         \
-        USE_TVM_OP=1                  \
-        USE_CPP_PACKAGE=1             \
-        USE_BLAS=openblas             \
-        USE_MKLDNN=0                  \
-        USE_DIST_KVSTORE=1            \
-        USE_LIBJPEG_TURBO=1           \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
-    make cython PYTHON=python3
+    pushd .
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_TVM_OP=ON \
+        -DUSE_CPP_PACKAGE=ON \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_MKLDNN=OFF \
+        -DUSE_CUDA=OFF \
+        -DUSE_DIST_KVSTORE=ON \
+        -DBUILD_CYTHON_MODULES=ON \
+        -DUSE_LIBJPEG_TURBO=ON \
+        -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_cpu_mkl() {
     set -ex
-    export CC="ccache gcc"
-    export CXX="ccache g++"
-    make \
-        DEV=1                         \
-        USE_CPP_PACKAGE=1             \
-        USE_BLAS=mkl                  \
-        USE_TVM_OP=1                  \
-        USE_MKLDNN=0                  \
-        USE_INTEL_PATH=/opt/intel     \
-        USE_DIST_KVSTORE=1            \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
+    pushd .
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_TVM_OP=ON \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-589883136
 
 
   Right, that makes sense.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421284
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -735,96 +713,93 @@ build_ubuntu_gpu_tensorrt() {
 
 build_ubuntu_gpu_mkldnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                                     \
-        USE_CPP_PACKAGE=1                         \
-        USE_BLAS=openblas                         \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=1                               \
-        USE_TVM_OP=1                              \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384630463
 
 

 ##########
 File path: cmake/Modules/FindMKL.cmake
 ##########
 @@ -122,10 +126,9 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains intel libs")
     list(APPEND MKL_LIBRARIES ${${__mkl_lib_upper}_LIBRARY})
   endforeach()
 
-
   if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY)
-    if (MKL_USE_STATIC_LIBS)
-      set(__iomp5_libs iomp5 libiomp5mt.lib)
+    if (MKL_USE_STATIC_LIBS AND UNIX)
+      set(__iomp5_libs libiomp5.a libiomp5mt.lib)
 
 Review comment:
   Otherwise symbols are not resolved at link time and the llvm openmp will be used. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384873729
 
 

 ##########
 File path: cmake/Modules/FindMKL.cmake
 ##########
 @@ -122,10 +126,9 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains intel libs")
     list(APPEND MKL_LIBRARIES ${${__mkl_lib_upper}_LIBRARY})
   endforeach()
 
-
   if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY)
-    if (MKL_USE_STATIC_LIBS)
-      set(__iomp5_libs iomp5 libiomp5mt.lib)
+    if (MKL_USE_STATIC_LIBS AND UNIX)
+      set(__iomp5_libs libiomp5.a libiomp5mt.lib)
 
 Review comment:
   @leezu, I'm thinking about static linking MKL libraries and leaving omp dynamic link to mxnet.so. It should not have any issue about unresolved symbol. But please note that if linking libiomp5.so or llvm libomp.so, the MKL library should be `libmkl_intel_thread.a`, otherwise if libgomp.so, the MKL library should be `libmkl_gnu_thread.a`. Refer to the changes to makefile: https://github.com/apache/incubator-mxnet/pull/17693/files

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] cjolivier01 commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
cjolivier01 commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384653232
 
 

 ##########
 File path: cmake/Modules/FindMKL.cmake
 ##########
 @@ -122,10 +126,9 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains intel libs")
     list(APPEND MKL_LIBRARIES ${${__mkl_lib_upper}_LIBRARY})
   endforeach()
 
-
   if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY)
-    if (MKL_USE_STATIC_LIBS)
-      set(__iomp5_libs iomp5 libiomp5mt.lib)
+    if (MKL_USE_STATIC_LIBS AND UNIX)
+      set(__iomp5_libs libiomp5.a libiomp5mt.lib)
 
 Review comment:
   Not in favor of tail wagging the dog solution, bending to allow mkl to behave foolishly.
   Also, this doesn;t help transitive dependencies, i.e. some other clang library has pulled in libomp, and then you still have the problem.  I believe the fix is to squash mkl's bad behavior, possily through static linking or some other way.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384757484
 
 

 ##########
 File path: 3rdparty/mshadow/mshadow/base.h
 ##########
 @@ -839,7 +840,7 @@ MSHADOW_XINLINE bool MaxValue<bool>(void) {
 /*! \brief maximum value of uint32_t */
 template<>
 MSHADOW_XINLINE uint32_t MaxValue<uint32_t>(void) {
-  return -1;
+  return std::numeric_limits<uint32_t>::max();
 
 Review comment:
   Thanks. I am actually not questioning this change which I think is correct. I am wondering if you have checked places where the return value of "-1" from this function is expected and if this will break that assumption.
    

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu edited a comment on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu edited a comment on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-589798230
 
 
   What do you mean? The CI is testing this PR. dev environment is for changes to the CI itself. This doesn't touch the CI itself.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421238
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -617,63 +595,63 @@ build_ubuntu_cpu_clang_tidy() {
 
 build_ubuntu_cpu_clang39_mkldnn() {
     set -ex
-
-    export CXX=clang++-3.9
-    export CC=clang-3.9
-
-    build_ccache_wrappers
-
-    make \
-        USE_CPP_PACKAGE=1             \
-        USE_BLAS=openblas             \
-        USE_OPENMP=0                  \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    CXX=clang++-3.9 CC=clang-3.9 cmake \
+       -DUSE_MKL_IF_AVAILABLE=OFF \
+       -DUSE_MKLDNN=ON \
+       -DUSE_CUDA=OFF \
+       -DUSE_CPP_PACKAGE=ON \
+       -DUSE_OPENMP=OFF \
+       -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_cpu_clang60_mkldnn() {
     set -ex
-
-    export CXX=clang++-6.0
-    export CC=clang-6.0
-
-    build_ccache_wrappers
-
-    make \
-        USE_CPP_PACKAGE=1             \
-        USE_BLAS=openblas             \
-        USE_OPENMP=1                  \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    CXX=clang++-6.0 CC=clang-6.0 cmake \
+       -DUSE_MKL_IF_AVAILABLE=OFF \
+       -DUSE_MKLDNN=ON \
+       -DUSE_CUDA=OFF \
+       -DUSE_CPP_PACKAGE=ON \
+       -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_cpu_mkldnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                         \
-        USE_CPP_PACKAGE=1             \
-        USE_TVM_OP=1                  \
-        USE_BLAS=openblas             \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+       -DUSE_MKL_IF_AVAILABLE=OFF \
+       -DUSE_TVM_OP=ON \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] sxjscience commented on pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
sxjscience commented on pull request #17645:
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-652216359


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] TaoLv commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
TaoLv commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384310491
 
 

 ##########
 File path: cmake/Modules/FindMKL.cmake
 ##########
 @@ -122,10 +126,9 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains intel libs")
     list(APPEND MKL_LIBRARIES ${${__mkl_lib_upper}_LIBRARY})
   endforeach()
 
-
   if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY)
-    if (MKL_USE_STATIC_LIBS)
-      set(__iomp5_libs iomp5 libiomp5mt.lib)
+    if (MKL_USE_STATIC_LIBS AND UNIX)
+      set(__iomp5_libs libiomp5.a libiomp5mt.lib)
 
 Review comment:
   Hmm, why we need static link libiomp5.a?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu edited a comment on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu edited a comment on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-591531288
 
 
   @cjolivier01 this PR fails without fixing OMP. That's because the CI previously didn't test cmake OMP.
   
   In any separate PR to fix OMP, CI won't test the fix.
   
   The fix in this PR is exactly what we discussed and agreed on. Please revisit your review as it's not technical. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421316
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -735,96 +713,93 @@ build_ubuntu_gpu_tensorrt() {
 
 build_ubuntu_gpu_mkldnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                                     \
-        USE_CPP_PACKAGE=1                         \
-        USE_BLAS=openblas                         \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=1                               \
-        USE_TVM_OP=1                              \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
+        -DUSE_CUDA=ON \
+        -DMXNET_CUDA_ARCH="$CI_CMAKE_CUDA_ARCH" \
+        -DUSE_CPP_PACKAGE=ON \
+        -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_gpu_mkldnn_nocudnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                                     \
-        USE_BLAS=openblas                         \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=0                               \
-        USE_TVM_OP=1                              \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
+        -DUSE_CUDA=ON \
+        -DMXNET_CUDA_ARCH="$CI_CMAKE_CUDA_ARCH" \
+        -DUSE_CUDNN=OFF \
+        -DUSE_CPP_PACKAGE=ON \
+        -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_gpu_cuda101_cudnn7() {
     set -ex
-    build_ccache_wrappers
-    make \
-        DEV=1                                     \
-        USE_BLAS=openblas                         \
-        USE_MKLDNN=0                              \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=1                               \
-        USE_TVM_OP=1                              \
-        USE_CPP_PACKAGE=1                         \
-        USE_DIST_KVSTORE=1                        \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
-    make cython PYTHON=python3
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421301
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -735,96 +713,93 @@ build_ubuntu_gpu_tensorrt() {
 
 build_ubuntu_gpu_mkldnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                                     \
-        USE_CPP_PACKAGE=1                         \
-        USE_BLAS=openblas                         \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=1                               \
-        USE_TVM_OP=1                              \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
+        -DUSE_CUDA=ON \
+        -DMXNET_CUDA_ARCH="$CI_CMAKE_CUDA_ARCH" \
+        -DUSE_CPP_PACKAGE=ON \
+        -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_gpu_mkldnn_nocudnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                                     \
-        USE_BLAS=openblas                         \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=0                               \
-        USE_TVM_OP=1                              \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu commented on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-589528150
 
 
   @apeforest The PR currently attempts to not change any of the 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.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421518
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -735,96 +713,93 @@ build_ubuntu_gpu_tensorrt() {
 
 build_ubuntu_gpu_mkldnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                                     \
-        USE_CPP_PACKAGE=1                         \
-        USE_BLAS=openblas                         \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=1                               \
-        USE_TVM_OP=1                              \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
+        -DUSE_CUDA=ON \
+        -DMXNET_CUDA_ARCH="$CI_CMAKE_CUDA_ARCH" \
+        -DUSE_CPP_PACKAGE=ON \
+        -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_gpu_mkldnn_nocudnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                                     \
-        USE_BLAS=openblas                         \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=0                               \
-        USE_TVM_OP=1                              \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
+        -DUSE_CUDA=ON \
+        -DMXNET_CUDA_ARCH="$CI_CMAKE_CUDA_ARCH" \
+        -DUSE_CUDNN=OFF \
+        -DUSE_CPP_PACKAGE=ON \
+        -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_gpu_cuda101_cudnn7() {
     set -ex
-    build_ccache_wrappers
-    make \
-        DEV=1                                     \
-        USE_BLAS=openblas                         \
-        USE_MKLDNN=0                              \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=1                               \
-        USE_TVM_OP=1                              \
-        USE_CPP_PACKAGE=1                         \
-        USE_DIST_KVSTORE=1                        \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
-    make cython PYTHON=python3
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=ON \
+        -DUSE_CUDA=ON \
+        -DMXNET_CUDA_ARCH="$CI_CMAKE_CUDA_ARCH" \
+        -DUSE_CUDNN=ON \
+        -DUSE_MKLDNN=OFF \
+        -DUSE_CPP_PACKAGE=ON \
+        -DBUILD_CYTHON_MODULES=ON \
+        -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_gpu_cuda101_cudnn7_mkldnn_cpp_test() {
     set -ex
-    build_ccache_wrappers
-    make \
-        DEV=1                                     \
-        USE_BLAS=openblas                         \
-        USE_MKLDNN=1                              \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=1                               \
-        USE_TVM_OP=0                              \
-        USE_CPP_PACKAGE=1                         \
-        USE_DIST_KVSTORE=1                        \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
-    make test USE_CPP_PACKAGE=1 -j$(nproc)
-    make cython PYTHON=python3
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=OFF \
+        -DUSE_CUDA=ON \
+        -DMXNET_CUDA_ARCH="$CI_CMAKE_CUDA_ARCH" \
+        -DUSE_CUDNN=ON \
+        -DUSE_MKLDNN=ON \
+        -DUSE_CPP_PACKAGE=ON \
+        -DBUILD_CYTHON_MODULES=ON \
+        -DUSE_DIST_KVSTORE=ON \
+        -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_gpu_cuda101_cudnn7_no_tvm_op() {
     set -ex
-    build_ccache_wrappers
-    make \
-        DEV=1                                     \
-        USE_BLAS=openblas                         \
-        USE_MKLDNN=0                              \
-        USE_CUDA=1                                \
-        USE_CUDA_PATH=/usr/local/cuda             \
-        USE_CUDNN=1                               \
-        USE_TVM_OP=0                              \
-        USE_CPP_PACKAGE=1                         \
-        USE_DIST_KVSTORE=1                        \
-        CUDA_ARCH="$CI_CUDA_COMPUTE_CAPABILITIES" \
-        USE_SIGNAL_HANDLER=1                      \
-        -j$(nproc)
-
-    make cython PYTHON=python3
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=OFF \
+        -DUSE_TVM_OP=OFF \
 
 Review comment:
   I think we should change this to TVM_OP = ON and rename this test to `build_ubuntu_gpu_cuda101_cudnn7_tvm_op()`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384757484
 
 

 ##########
 File path: 3rdparty/mshadow/mshadow/base.h
 ##########
 @@ -839,7 +840,7 @@ MSHADOW_XINLINE bool MaxValue<bool>(void) {
 /*! \brief maximum value of uint32_t */
 template<>
 MSHADOW_XINLINE uint32_t MaxValue<uint32_t>(void) {
-  return -1;
+  return std::numeric_limits<uint32_t>::max();
 
 Review comment:
   Thanks. I am actually not questioning this change which I think is correct. I am wondering if you have checked places where the return value of "-1" from this function is expected and if this will break that assumption.
    

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu commented on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-591531288
 
 
   @cjolivier01 this PR fails without fixing OMP. That's because the CI previously didn't test cmake OMP.
   The fix in this PR is exactly what we discussed and agreed on. Please revisit your review as it's not technical. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384655883
 
 

 ##########
 File path: cmake/Modules/FindMKL.cmake
 ##########
 @@ -122,10 +126,9 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains intel libs")
     list(APPEND MKL_LIBRARIES ${${__mkl_lib_upper}_LIBRARY})
   endforeach()
 
-
   if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY)
-    if (MKL_USE_STATIC_LIBS)
-      set(__iomp5_libs iomp5 libiomp5mt.lib)
+    if (MKL_USE_STATIC_LIBS AND UNIX)
+      set(__iomp5_libs libiomp5.a libiomp5mt.lib)
 
 Review comment:
   @cjolivier01 I'll remove testing MKL builds from this PR and you can contribute a separate PR that fixes MKL build and enables tests.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu merged pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu merged pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu edited a comment on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu edited a comment on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-591552783
 
 
   @cjolivier01 I'll remove the MKL fix and testing MKL builds from this PR and you can contribute a separate PR that fixes MKL build and enables tests based on your preferred solution.
   
   Please remove your veto.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421639
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -855,7 +830,6 @@ build_ubuntu_gpu_cmake_mkldnn() {
         -DUSE_CUDA=1                            \
         -DUSE_CUDNN=1                           \
         -DUSE_TVM_OP=1                          \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu commented on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-591552783
 
 
   @cjolivier01 I'll remove the MKL fix and testing MKL builds from this PR and you can contribute a separate PR that fixes MKL build and enables tests.
   
   Please remove your veto.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421259
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -617,63 +595,63 @@ build_ubuntu_cpu_clang_tidy() {
 
 build_ubuntu_cpu_clang39_mkldnn() {
     set -ex
-
-    export CXX=clang++-3.9
-    export CC=clang-3.9
-
-    build_ccache_wrappers
-
-    make \
-        USE_CPP_PACKAGE=1             \
-        USE_BLAS=openblas             \
-        USE_OPENMP=0                  \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    CXX=clang++-3.9 CC=clang-3.9 cmake \
+       -DUSE_MKL_IF_AVAILABLE=OFF \
+       -DUSE_MKLDNN=ON \
+       -DUSE_CUDA=OFF \
+       -DUSE_CPP_PACKAGE=ON \
+       -DUSE_OPENMP=OFF \
+       -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_cpu_clang60_mkldnn() {
     set -ex
-
-    export CXX=clang++-6.0
-    export CC=clang-6.0
-
-    build_ccache_wrappers
-
-    make \
-        USE_CPP_PACKAGE=1             \
-        USE_BLAS=openblas             \
-        USE_OPENMP=1                  \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    CXX=clang++-6.0 CC=clang-6.0 cmake \
+       -DUSE_MKL_IF_AVAILABLE=OFF \
+       -DUSE_MKLDNN=ON \
+       -DUSE_CUDA=OFF \
+       -DUSE_CPP_PACKAGE=ON \
+       -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_cpu_mkldnn() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                         \
-        USE_CPP_PACKAGE=1             \
-        USE_TVM_OP=1                  \
-        USE_BLAS=openblas             \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+       -DUSE_MKL_IF_AVAILABLE=OFF \
+       -DUSE_TVM_OP=ON \
+       -DUSE_MKLDNN=ON \
+       -DUSE_CUDA=OFF \
+       -DUSE_CPP_PACKAGE=ON \
+       -G Ninja /work/mxnet
+    ninja
+    popd
 }
 
 build_ubuntu_cpu_mkldnn_mkl() {
     set -ex
-
-    build_ccache_wrappers
-
-    make  \
-        DEV=1                         \
-        USE_CPP_PACKAGE=1             \
-        USE_TVM_OP=1                  \
-        USE_BLAS=mkl                  \
-        USE_SIGNAL_HANDLER=1          \
-        USE_INTEL_PATH=/opt/intel/    \
-        -j$(nproc)
+    pushd .
+    cd /work/build
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_MKL_IF_AVAILABLE=ON \
+        -DBLAS="MKL" \
+        -DUSE_TVM_OP=ON \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384659704
 
 

 ##########
 File path: cmake/Modules/FindMKL.cmake
 ##########
 @@ -122,10 +126,9 @@ set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains intel libs")
     list(APPEND MKL_LIBRARIES ${${__mkl_lib_upper}_LIBRARY})
   endforeach()
 
-
   if(NOT MKL_USE_SINGLE_DYNAMIC_LIBRARY)
-    if (MKL_USE_STATIC_LIBS)
-      set(__iomp5_libs iomp5 libiomp5mt.lib)
+    if (MKL_USE_STATIC_LIBS AND UNIX)
+      set(__iomp5_libs libiomp5.a libiomp5mt.lib)
 
 Review comment:
   Btw, if you look a few line above, you see that we are linking MKL statically.
   Even if MKL is statically linked, dynamic linker may load llvm openmp as long as llvm openmp is pulled in by clang (depending on the link order)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r384734352
 
 

 ##########
 File path: 3rdparty/mshadow/mshadow/base.h
 ##########
 @@ -839,7 +840,7 @@ MSHADOW_XINLINE bool MaxValue<bool>(void) {
 /*! \brief maximum value of uint32_t */
 template<>
 MSHADOW_XINLINE uint32_t MaxValue<uint32_t>(void) {
-  return -1;
+  return std::numeric_limits<uint32_t>::max();
 
 Review comment:
   What's the indication of this change? Would it break some existing logic?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421089
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -455,36 +437,40 @@ build_ubuntu_cpu() {
 
 build_ubuntu_cpu_openblas() {
     set -ex
-    export CC="gcc"
-    export CXX="g++"
-    build_ccache_wrappers
-    make \
-        DEV=1                         \
-        USE_TVM_OP=1                  \
-        USE_CPP_PACKAGE=1             \
-        USE_BLAS=openblas             \
-        USE_MKLDNN=0                  \
-        USE_DIST_KVSTORE=1            \
-        USE_LIBJPEG_TURBO=1           \
-        USE_SIGNAL_HANDLER=1          \
-        -j$(nproc)
-    make cython PYTHON=python3
+    pushd .
+    cmake \
+        -DCMAKE_BUILD_TYPE="RelWithDebInfo" \
+        -DUSE_TVM_OP=ON \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-589791922
 
 
   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?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#discussion_r382421670
 
 

 ##########
 File path: ci/docker/runtime_functions.sh
 ##########
 @@ -897,7 +870,6 @@ build_ubuntu_gpu_cmake_no_rtc() {
         -DUSE_CUDA=ON                           \
         -DUSE_CUDNN=ON                          \
         -DUSE_TVM_OP=ON                         \
 
 Review comment:
   I think we should turn off this by default and create a separate to test TVM_ON instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] leezu commented on issue #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
leezu commented on issue #17645: [CI] Test CMake builds instead of Makefile builds
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-589798230
 
 
   What do you mean? The CI is testing this PR. This has noting to do with the dev environment?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-mxnet] ciyongch commented on pull request #17645: [CI] Test CMake builds instead of Makefile builds

Posted by GitBox <gi...@apache.org>.
ciyongch commented on pull request #17645:
URL: https://github.com/apache/incubator-mxnet/pull/17645#issuecomment-653370729


   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.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org