[GitHub] larroy commented on a change in pull request #11055: [MXNET-57] Added support android64
larroy commented on a change in pull request #11055: [MXNET-57] Added support android64 URL: https://github.com/apache/incubator-mxnet/pull/11055#discussion_r194963500 ## File path: ci/docker/runtime_functions.sh ## @@ -163,6 +163,30 @@ build_android_arm64() { cp dist/*.whl /work/build } +build_android_arm64() { +set -ex +cd /work/build +#-DCMAKE_ANDROID_NDK=${CROSS_ROOT} \ +#-DCMAKE_SYSTEM_VERSION=${ANDROID_NDK_REVISION} \ +#-DCMAKE_SYSTEM_NAME=Android \ +cmake\ +-DANDROID=ON \ +-DUSE_CUDA=OFF\ +-DUSE_SSE=OFF\ +-DUSE_LAPACK=OFF\ +-DUSE_OPENCV=OFF\ +-DUSE_OPENMP=OFF\ +-DUSE_SIGNAL_HANDLER=ON\ +-DCMAKE_BUILD_TYPE=RelWithDebInfo\ Review comment: I don't see how the points you are making about an experimental build which is not official nor tested are of any relevance at this stage of Android support. This is a development build and I want debug symbols to diagnose crashes, this should be reason enough to have it as it is, because size of build doesn't really matter right now, and debugability is more important. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] larroy commented on a change in pull request #11055: [MXNET-57] Added support android64
larroy commented on a change in pull request #11055: [MXNET-57] Added support android64 URL: https://github.com/apache/incubator-mxnet/pull/11055#discussion_r194849319 ## File path: ci/docker/Dockerfile.build.android_arm64 ## @@ -18,23 +18,20 @@ # # Dockerfile to build MXNet for Android ARM64/ARMv8 -FROM ubuntu:16.04 as ccachebuilder - -COPY install/ubuntu_core.sh /work/ -RUN /work/ubuntu_core.sh -COPY install/ubuntu_ccache.sh /work/ -RUN /work/ubuntu_ccache.sh +#FROM ubuntu:16.04 as ccachebuilder +#COPY install/ubuntu_core.sh /work/ +#RUN /work/ubuntu_core.sh +#COPY install/ubuntu_ccache.sh /work/ +#RUN /work/ubuntu_ccache.sh Review comment: Let's get the ARMv7 PR merged first and then I ammend this one and re-enable the ccache logic. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] larroy commented on a change in pull request #11055: [MXNET-57] Added support android64
larroy commented on a change in pull request #11055: [MXNET-57] Added support android64 URL: https://github.com/apache/incubator-mxnet/pull/11055#discussion_r194761222 ## File path: CMakeLists.txt ## @@ -15,9 +15,9 @@ mxnet_option(USE_NCCL "Use NVidia NCCL with CUDA" OFF) mxnet_option(USE_OPENCV "Build with OpenCV support" ON) mxnet_option(USE_OPENMP "Build with Openmp support" ON) mxnet_option(USE_CUDNN"Build with cudnn support" ON) # one could set CUDNN_ROOT for search path -mxnet_option(USE_SSE "Build with x86 SSE instruction support" ON) +mxnet_option(USE_SSE "Build with x86 SSE instruction support" ON IF NOT ARM) mxnet_option(USE_F16C "Build with x86 F16C instruction support" ON) # autodetects support if ON Review comment: Can you give thumbs up or down to the review? Any more comments? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] larroy commented on a change in pull request #11055: [MXNET-57] Added support android64
larroy commented on a change in pull request #11055: [MXNET-57] Added support android64 URL: https://github.com/apache/incubator-mxnet/pull/11055#discussion_r194704386 ## File path: ci/docker/Dockerfile.build.android_arm64 ## @@ -18,23 +18,20 @@ # # Dockerfile to build MXNet for Android ARM64/ARMv8 -FROM ubuntu:16.04 as ccachebuilder - -COPY install/ubuntu_core.sh /work/ -RUN /work/ubuntu_core.sh -COPY install/ubuntu_ccache.sh /work/ -RUN /work/ubuntu_ccache.sh +#FROM ubuntu:16.04 as ccachebuilder +#COPY install/ubuntu_core.sh /work/ +#RUN /work/ubuntu_core.sh +#COPY install/ubuntu_ccache.sh /work/ +#RUN /work/ubuntu_ccache.sh FROM dockcross/base:latest Review comment: Happy to do so in a subsequent PR, now this one passed. I would very much like to get it merged so at least we have this build as a gatekeeping mechanism. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] larroy commented on a change in pull request #11055: [MXNET-57] Added support android64
larroy commented on a change in pull request #11055: [MXNET-57] Added support android64 URL: https://github.com/apache/incubator-mxnet/pull/11055#discussion_r194337512 ## File path: ci/docker/runtime_functions.sh ## @@ -163,6 +163,30 @@ build_android_arm64() { cp dist/*.whl /work/build } +build_android_arm64() { +set -ex +cd /work/build +#-DCMAKE_ANDROID_NDK=${CROSS_ROOT} \ +#-DCMAKE_SYSTEM_VERSION=${ANDROID_NDK_REVISION} \ +#-DCMAKE_SYSTEM_NAME=Android \ +cmake\ +-DANDROID=ON \ +-DUSE_CUDA=OFF\ +-DUSE_SSE=OFF\ +-DUSE_LAPACK=OFF\ +-DUSE_OPENCV=OFF\ +-DUSE_OPENMP=OFF\ Review comment: This is an initial build, what you are saying doesn't apply. Nobody should test this. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services