-
Notifications
You must be signed in to change notification settings - Fork 230
Add smoke test for binary package #442
Conversation
687b13a
to
68407ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally lgtm, just some nits.
windows/internal/smoke_test.bat
Outdated
set LIB=%LIB%;%install_root%/lib | ||
set PATH=%PATH%;%install_root%/lib | ||
|
||
>example-app.cpp ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Consider creating a new file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the name of the executable is not clear.
windows/internal/smoke_test.bat
Outdated
.\example-app.exe | ||
if ERRORLEVEL 1 exit /b 1 | ||
|
||
echo Checking that MKL is available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
windows/internal/smoke_test.bat
Outdated
|
||
del example-app.cpp | ||
>example-app.cpp ( | ||
echo #include ^<torch/torch.h^> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
windows/internal/smoke_test.bat
Outdated
conda create -qyn testenv python=%DESIRED_PYTHON% | ||
source activate testenv >/dev/null | ||
for /F "delims=" %%i in ('where /R "%PYTORCH_FINAL_PACKAGE_DIR:/=\%" *.tar.bz2') do conda install -y "%%i" --offline | ||
conda install -yq future numpy protobuf six |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Check the exitcode here?
windows/internal/smoke_test.bat
Outdated
set CUDA_VER_MAJOR=%CUDA_VERSION:~0,-1% | ||
set CUDA_VER_MINOR=%CUDA_VERSION:~-1,1% | ||
set CUDA_VERSION_STR=%CUDA_VER_MAJOR%.%CUDA_VER_MINOR% | ||
conda install -yq -c pytorch "cudatoolkit=%CUDA_VERSION_STR%" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Check the exitcode here?
windows/internal/smoke_test.bat
Outdated
powershell internal/vs_install.ps1 | ||
if ERRORLEVEL 1 exit /b 1 | ||
|
||
for /F "delims=" %%i in ('where /R "%PYTORCH_FINAL_PACKAGE_DIR:/=\%" *-latest.zip') do unzip "%%i" -d tmp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Consider using 7z instead. unzip
is only available in Bash. The equivalent line using 7z is 7z x "%%i" -otmp
.
windows/internal/smoke_test.bat
Outdated
set "PATH=%CD%\Python%PYTHON_VERSION%\Scripts;%CD%\Python%PYTHON_VERSION%;%PATH%" | ||
|
||
for /F "delims=" %%i in ('where /R "%PYTORCH_FINAL_PACKAGE_DIR:/=\%" *.whl') do pip install "%%i" | ||
pip install -q future numpy protobuf six "mkl>=2019" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Check exitcode here?
windows/internal/smoke_test.bat
Outdated
set "PATH=%CONDA_HOME%;%CONDA_HOME%\scripts;%CONDA_HOME%\Library\bin;%PATH%" | ||
|
||
conda create -qyn testenv python=%DESIRED_PYTHON% | ||
source activate testenv >/dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again this is Bash
only, please use call %CONDA_HOME%\condabin\activate.bat testenv > NUL
.
cb42869
to
7318044
Compare
windows/internal/update_driver.bat
Outdated
@@ -0,0 +1,4 @@ | |||
set "DRIVER_DOWNLOAD_LINK=https://s3.amazonaws.com/ossci-windows/442.50-tesla-desktop-winserver-2019-2016-international.exe" | |||
curl -k -L DRIVER_DOWNLOAD_LINK --output 442.50-tesla-desktop-winserver-2019-2016-international.exe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
- check error code
- add
--retry
option
windows/internal/update_driver.bat
Outdated
set "DRIVER_DOWNLOAD_LINK=https://s3.amazonaws.com/ossci-windows/442.50-tesla-desktop-winserver-2019-2016-international.exe" | ||
curl -k -L DRIVER_DOWNLOAD_LINK --output 442.50-tesla-desktop-winserver-2019-2016-international.exe | ||
|
||
start /wait 442.50-tesla-desktop-winserver-2019-2016-international.exe -s -noreboot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: check exit code
windows/internal/update_driver.bat
Outdated
@@ -0,0 +1,4 @@ | |||
set "DRIVER_DOWNLOAD_LINK=https://s3.amazonaws.com/ossci-windows/442.50-tesla-desktop-winserver-2019-2016-international.exe" | |||
curl -k -L DRIVER_DOWNLOAD_LINK --output 442.50-tesla-desktop-winserver-2019-2016-international.exe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curl -k -L DRIVER_DOWNLOAD_LINK --output 442.50-tesla-desktop-winserver-2019-2016-international.exe | |
curl --retry 3 -kL "%DRIVER_DOWNLOAD_LINK%" --output 442.50-tesla-desktop-winserver-2019-2016-international.exe | |
if errorlevel 1 exit /b 1 |
6492ccc
to
e4a05e3
Compare
check_binary.sh
Outdated
@@ -256,8 +256,8 @@ build_and_run_example_cpp () { | |||
if [ -f "${install_root}/lib/libtorch_cuda.so" ] || [ -f "${install_root}/lib/libtorch_cuda.dylib" ]; then | |||
TORCH_CUDA_LINK_FLAGS="-ltorch_cuda" | |||
fi | |||
g++ example-app.cpp -I${install_root}/include -I${install_root}/include/torch/csrc/api/include -D_GLIBCXX_USE_CXX11_ABI=$GLIBCXX_USE_CXX11_ABI -std=gnu++14 -L${install_root}/lib ${REF_LIB} ${ADDITIONAL_LINKER_FLAGS} -ltorch $TORCH_CPU_LINK_FLAGS $TORCH_CUDA_LINK_FLAGS $C10_LINK_FLAGS -o example-app | |||
./example-app | |||
g++ ${BUILDER_ROOT}/test_example_code/$1.cpp -I${install_root}/include -I${install_root}/include/torch/csrc/api/include -D_GLIBCXX_USE_CXX11_ABI=$GLIBCXX_USE_CXX11_ABI -std=gnu++14 -L${install_root}/lib ${REF_LIB} ${ADDITIONAL_LINKER_FLAGS} -ltorch $TORCH_CPU_LINK_FLAGS $TORCH_CUDA_LINK_FLAGS $C10_LINK_FLAGS -o $1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is $1
used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is
$1
used here?
The code file name is passed as argument from outside.
windows/internal/driver_update.bat
Outdated
if errorlevel 1 exit /b 1 | ||
|
||
del 442.50-tesla-desktop-winserver-2019-2016-international.exe || ver > NUL | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: empty line?
Could you please take some time to review this PR, @seemethere? |
@@ -256,8 +256,8 @@ build_and_run_example_cpp () { | |||
if [ -f "${install_root}/lib/libtorch_cuda.so" ] || [ -f "${install_root}/lib/libtorch_cuda.dylib" ]; then | |||
TORCH_CUDA_LINK_FLAGS="-ltorch_cuda" | |||
fi | |||
g++ example-app.cpp -I${install_root}/include -I${install_root}/include/torch/csrc/api/include -D_GLIBCXX_USE_CXX11_ABI=$GLIBCXX_USE_CXX11_ABI -std=gnu++14 -L${install_root}/lib ${REF_LIB} ${ADDITIONAL_LINKER_FLAGS} -ltorch $TORCH_CPU_LINK_FLAGS $TORCH_CUDA_LINK_FLAGS $C10_LINK_FLAGS -o example-app | |||
./example-app | |||
g++ /builder/test_example_code/$1.cpp -I${install_root}/include -I${install_root}/include/torch/csrc/api/include -D_GLIBCXX_USE_CXX11_ABI=$GLIBCXX_USE_CXX11_ABI -std=gnu++14 -L${install_root}/lib ${REF_LIB} ${ADDITIONAL_LINKER_FLAGS} -ltorch $TORCH_CPU_LINK_FLAGS $TORCH_CUDA_LINK_FLAGS $C10_LINK_FLAGS -o $1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, just noticed that this is an absolute path.
@@ -287,7 +287,7 @@ build_example_cpp_with_incorrect_abi () { | |||
if [ -f "${install_root}/lib/libtorch_cuda.so" ] || [ -f "${install_root}/lib/libtorch_cuda.dylib" ]; then | |||
TORCH_CUDA_LINK_FLAGS="-ltorch_cuda" | |||
fi | |||
g++ example-app.cpp -I${install_root}/include -I${install_root}/include/torch/csrc/api/include -D_GLIBCXX_USE_CXX11_ABI=$GLIBCXX_USE_CXX11_ABI -std=gnu++14 -L${install_root}/lib ${REF_LIB} ${ADDITIONAL_LINKER_FLAGS} -ltorch $TORCH_CPU_LINK_FLAGS $TORCH_CUDA_LINK_FLAGS $C10_LINK_FLAGS -o example-app | |||
g++ /builder/test_example_code/$1.cpp -I${install_root}/include -I${install_root}/include/torch/csrc/api/include -D_GLIBCXX_USE_CXX11_ABI=$GLIBCXX_USE_CXX11_ABI -std=gnu++14 -L${install_root}/lib ${REF_LIB} ${ADDITIONAL_LINKER_FLAGS} -ltorch $TORCH_CPU_LINK_FLAGS $TORCH_CUDA_LINK_FLAGS $C10_LINK_FLAGS -o $1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this
No description provided.