[GitHub] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r184509001
##
File path: cmake/FirstClassLangCuda.cmake
##
@@ -126,7 +126,7 @@ endif ()
function(mshadow_select_nvcc_arch_flags out_variable)
set(CUDA_ARCH_LIST "Auto" CACHE STRING "Select target NVIDIA GPU
achitecture.")
- set_property( CACHE CUDA_ARCH_LIST PROPERTY STRINGS "" "All" "Common"
${CUDA_KNOWN_GPU_ARCHITECTURES} )
+ set_property( CACHE CUDA_ARCH_LIST PROPERTY STRINGS "" "Auto" "All" "Common"
${CUDA_KNOWN_GPU_ARCHITECTURES} )
Review comment:
This is a default behaviour change, right?
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183235642
##
File path: cmake/MklDnn.cmake
##
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#this file download mklml
+
+message(STATUS "download mklml")
+if(MSVC)
+ set(MKL_NAME "mklml_win_2018.0.2.20180127")
+ file(DOWNLOAD
"https://github.com/intel/mkl-dnn/releases/download/v0.13/${MKL_NAME}.zip;
"${CMAKE_CURRENT_BINARY_DIR}/mklml/${MKL_NAME}.zip" EXPECTED_MD5
"D7B4BD9E85BB7B85F5C956FE119A8722" SHOW_PROGRESS)
+ file(DOWNLOAD
"https://github.com/yajiedesign/mxnet/releases/download/weekly_binary_build_v2/7z.exe;
"${CMAKE_CURRENT_BINARY_DIR}/mklml/7z2.exe" EXPECTED_MD5
"E1CF766CF358F368EC97662D06EA5A4C" SHOW_PROGRESS)
Review comment:
Also please refrain from downloading unstable weekly builds. We want our
users to only use stable and released versions.
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183235611
##
File path: cmake/MklDnn.cmake
##
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#this file download mklml
+
+message(STATUS "download mklml")
+if(MSVC)
+ set(MKL_NAME "mklml_win_2018.0.2.20180127")
+ file(DOWNLOAD
"https://github.com/intel/mkl-dnn/releases/download/v0.13/${MKL_NAME}.zip;
"${CMAKE_CURRENT_BINARY_DIR}/mklml/${MKL_NAME}.zip" EXPECTED_MD5
"D7B4BD9E85BB7B85F5C956FE119A8722" SHOW_PROGRESS)
+ file(DOWNLOAD
"https://github.com/yajiedesign/mxnet/releases/download/weekly_binary_build_v2/7z.exe;
"${CMAKE_CURRENT_BINARY_DIR}/mklml/7z2.exe" EXPECTED_MD5
"E1CF766CF358F368EC97662D06EA5A4C" SHOW_PROGRESS)
Review comment:
Please refrain from using private repositories
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183235548
##
File path: Jenkinsfile
##
@@ -311,7 +311,7 @@ try {
bat """mkdir build_vc14_gpu
call "C:\\Program Files (x86)\\Microsoft Visual Studio
14.0\\VC\\bin\\x86_amd64\\vcvarsx86_amd64.bat"
cd build_vc14_gpu
- cmake -G \"NMake Makefiles JOM\" -DUSE_CUDA=1 -DUSE_CUDNN=1
-DUSE_NVRTC=1 -DUSE_OPENCV=1 -DUSE_OPENMP=1 -DUSE_PROFILER=1 -DUSE_BLAS=open
-DUSE_LAPACK=1 -DUSE_DIST_KVSTORE=0 -DCUDA_ARCH_NAME=All
-DCMAKE_CXX_FLAGS_RELEASE="/FS /MD /O2 /Ob2 /DNDEBUG"
-DCMAKE_BUILD_TYPE=Release ${env.WORKSPACE}"""
+ cmake -G \"NMake Makefiles JOM\" -DUSE_CUDA=1 -DUSE_CUDNN=1
-DUSE_NVRTC=1 -DUSE_OPENCV=1 -DUSE_OPENMP=1 -DUSE_PROFILER=1 -DUSE_BLAS=open
-DUSE_LAPACK=1 -DUSE_DIST_KVSTORE=0 -DCUDA_ARCH_NAME=Maxwell
-DCMAKE_CXX_FLAGS_RELEASE="/FS /MD /O2 /Ob2 /DNDEBUG"
-DCMAKE_BUILD_TYPE=Release -DUSE_MKL_IF_AVAILABLE=0 ${env.WORKSPACE}"""
Review comment:
Please leave the cuda-archs unchanged
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183235469
##
File path: Jenkinsfile
##
@@ -622,6 +663,42 @@ try {
}
}
}
+},
+'Python 2: MKLDNN-GPU Win':{
+ node('mxnetwindows-gpu') {
+timeout(time: max_time, unit: 'MINUTES') {
+ ws('workspace/ut-python-gpu') {
+ init_git_win()
+ unstash 'vc14_gpu_mkldnn'
+ bat '''rmdir /s/q pkg_vc14_gpu_mkldnn
+7z x -y vc14_gpu_mkldnn.7z'''
+ bat """xcopy C:\\mxnet\\data data /E /I /Y
+xcopy C:\\mxnet\\model model /E /I /Y
+call activate py2
+set PYTHONPATH=${env.WORKSPACE}\\pkg_vc14_gpu_mkldnn\\python
+del /S /Q ${env.WORKSPACE}\\pkg_vc14_gpu_mkldnn\\python\\*.pyc
+C:\\mxnet\\test_gpu.bat"""
+ }
+}
+ }
+},
+'Python 3: MKLDNN-GPU Win':{
Review comment:
Yes, please only keep one test. Since you're testing the CPP backend, which
is independent of Python, there's no need to test both versions.
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183215290
##
File path: python/mxnet/libinfo.py
##
@@ -69,6 +72,8 @@ def find_lib_path():
if len(lib_path) == 0:
raise RuntimeError('Cannot find the MXNet library.\n' +
'List of candidates:\n' + str('\n'.join(dll_path)))
+if os.name == 'nt':
+os.environ['PATH'] = os.path.dirname(lib_path[0]) + ';' +
os.environ['PATH']
Review comment:
same here
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183215285
##
File path: python/mxnet/libinfo.py
##
@@ -37,6 +38,8 @@ def find_lib_path():
logging.warning("MXNET_LIBRARY_PATH should be an absolute
path, instead of: %s",
lib_from_env)
else:
+if os.name == 'nt':
+os.environ['PATH'] = os.path.dirname(lib_from_env) + ';' +
os.environ['PATH']
Review comment:
Please append to the end of path instead of the beginning to prevent path
masking
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183215251
##
File path: cmake/FirstClassLangCuda.cmake
##
@@ -125,8 +125,8 @@ endif ()
# mshadow_select_nvcc_arch_flags(out_variable)
function(mshadow_select_nvcc_arch_flags out_variable)
- set(CUDA_ARCH_LIST "Auto" CACHE STRING "Select target NVIDIA GPU
achitecture.")
- set_property( CACHE CUDA_ARCH_LIST PROPERTY STRINGS "" "All" "Common"
${CUDA_KNOWN_GPU_ARCHITECTURES} )
+ set(CUDA_ARCH_LIST "" CACHE STRING "Select target NVIDIA GPU achitecture.")
Review comment:
default behaviour change?
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183215222
##
File path: Jenkinsfile
##
@@ -334,6 +334,47 @@ try {
}
}
},
+'Build GPU MKLDNN windows':{
+ node('mxnetwindows-cpu') {
+timeout(time: max_time, unit: 'MINUTES') {
+ ws('workspace/build-gpu') {
+withEnv(['OpenBLAS_HOME=C:\\mxnet\\openblas',
'OpenCV_DIR=C:\\mxnet\\opencv_vc14',
'CUDA_PATH=C:\\CUDA\\v8.0','BUILD_NAME=vc14_gpu_mkldnn']) {
+init_git_win()
+bat """mkdir build_%BUILD_NAME%
+ call "C:\\Program Files (x86)\\Microsoft Visual Studio
14.0\\VC\\bin\\x86_amd64\\vcvarsx86_amd64.bat"
+ cd build_%BUILD_NAME%
+ copy
${env.WORKSPACE}\\3rdparty\\mkldnn\\config_template.vcxproj.user
${env.WORKSPACE}\\config_template.vcxproj.user /y
+ cmake -G \"NMake Makefiles JOM\" -DUSE_CUDA=1 -DUSE_CUDNN=1
-DUSE_NVRTC=1 -DUSE_OPENCV=1 -DUSE_OPENMP=1 -DUSE_PROFILER=1 -DUSE_BLAS=open
-DUSE_LAPACK=1 -DUSE_DIST_KVSTORE=0 -DCUDA_ARCH_NAME=Maxwell -DUSE_MKLDNN=1
-DCMAKE_CXX_FLAGS_RELEASE="/FS /MD /O2 /Ob2 /DNDEBUG"
-DCMAKE_BUILD_TYPE=Release ${env.WORKSPACE}"""
+bat '''
+call "C:\\Program Files (x86)\\Microsoft Visual Studio
14.0\\VC\\bin\\x86_amd64\\vcvarsx86_amd64.bat"
+cd build_%BUILD_NAME%
+set /a cores=36 * 2
+jom -j 72
Review comment:
Please use the proper env variables and don't hardcode these values
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn
with msvc
URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183215179
##
File path: Jenkinsfile
##
@@ -311,7 +311,7 @@ try {
bat """mkdir build_vc14_gpu
call "C:\\Program Files (x86)\\Microsoft Visual Studio
14.0\\VC\\bin\\x86_amd64\\vcvarsx86_amd64.bat"
cd build_vc14_gpu
- cmake -G \"NMake Makefiles JOM\" -DUSE_CUDA=1 -DUSE_CUDNN=1
-DUSE_NVRTC=1 -DUSE_OPENCV=1 -DUSE_OPENMP=1 -DUSE_PROFILER=1 -DUSE_BLAS=open
-DUSE_LAPACK=1 -DUSE_DIST_KVSTORE=0 -DCUDA_ARCH_NAME=All
-DCMAKE_CXX_FLAGS_RELEASE="/FS /MD /O2 /Ob2 /DNDEBUG"
-DCMAKE_BUILD_TYPE=Release ${env.WORKSPACE}"""
+ cmake -G \"NMake Makefiles JOM\" -DUSE_CUDA=1 -DUSE_CUDNN=1
-DUSE_NVRTC=1 -DUSE_OPENCV=1 -DUSE_OPENMP=1 -DUSE_PROFILER=1 -DUSE_BLAS=open
-DUSE_LAPACK=1 -DUSE_DIST_KVSTORE=0 -DCUDA_ARCH_NAME=Maxwell
-DCMAKE_CXX_FLAGS_RELEASE="/FS /MD /O2 /Ob2 /DNDEBUG"
-DCMAKE_BUILD_TYPE=Release ${env.WORKSPACE}"""
Review comment:
Please elaborate this change? We're trying to cover all supported archs
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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183215160 ## File path: CMakeLists.txt ## @@ -18,8 +18,8 @@ mxnet_option(USE_CUDNN"Build with cudnn support" ON) # one could se mxnet_option(USE_SSE "Build with x86 SSE instruction support" ON) mxnet_option(USE_LAPACK "Build with lapack support" ON IF NOT MSVC) mxnet_option(USE_MKL_IF_AVAILABLE "Use MKL if found" ON) -mxnet_option(USE_MKLML_MKL"Use MKLDNN variant of MKL (if MKL found)" ON IF USE_MKL_IF_AVAILABLE AND UNIX AND (NOT APPLE)) -mxnet_option(USE_MKLDNN "Use MKLDNN variant of MKL (if MKL found)" ON IF USE_MKL_IF_AVAILABLE AND UNIX AND (NOT APPLE)) +mxnet_option(USE_MKLML_MKL"Use MKLDNN variant of MKL (if MKL found)" OFF IF (NOT APPLE)) Review comment: Please keep the USE_MKL_IF_AVAILABLE argument - you're changing the default behaviour here 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] marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc
marcoabreu commented on a change in pull request #10629: [MXNET-343]fix Mkldnn with msvc URL: https://github.com/apache/incubator-mxnet/pull/10629#discussion_r183215160 ## File path: CMakeLists.txt ## @@ -18,8 +18,8 @@ mxnet_option(USE_CUDNN"Build with cudnn support" ON) # one could se mxnet_option(USE_SSE "Build with x86 SSE instruction support" ON) mxnet_option(USE_LAPACK "Build with lapack support" ON IF NOT MSVC) mxnet_option(USE_MKL_IF_AVAILABLE "Use MKL if found" ON) -mxnet_option(USE_MKLML_MKL"Use MKLDNN variant of MKL (if MKL found)" ON IF USE_MKL_IF_AVAILABLE AND UNIX AND (NOT APPLE)) -mxnet_option(USE_MKLDNN "Use MKLDNN variant of MKL (if MKL found)" ON IF USE_MKL_IF_AVAILABLE AND UNIX AND (NOT APPLE)) +mxnet_option(USE_MKLML_MKL"Use MKLDNN variant of MKL (if MKL found)" OFF IF (NOT APPLE)) Review comment: Please keep the USE_MKL_IF_AVAILABLE argument 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
