gemini-code-assist[bot] commented on code in PR #252: URL: https://github.com/apache/tvm-ffi/pull/252#discussion_r2508667327
########## addons/torch_c_dlpack_ext/build_aot_wheels.bat: ########## @@ -0,0 +1,88 @@ +@echo off +setlocal enabledelayedexpansion + +set arch=%~1 +set python_version=%~2 + +set tvm_ffi=%cd% +set torch_c_dlpack_ext=%tvm_ffi%\addons\torch_c_dlpack_ext + +for %%P in (2.4 2.5 2.6 2.7 2.8 2.9) do ( + call :build_libs %%P +) + +copy %tvm_ffi%\lib\*.dll %torch_c_dlpack_ext%\torch_c_dlpack_ext +uv venv %tvm_ffi%\.venv\build --python %python_version% +call %tvm_ffi%\.venv\build\Scripts\activate +uv pip install build wheel auditwheel +cd %torch_c_dlpack_ext% +python -m build -w +dir dist +for %%f in (dist\*.whl) do python -m wheel tags "%%f" --python-tag=%python_version% --abi-tag=%python_version% --platform-tag=win_amd64 +dir dist +mkdir wheelhouse +copy dist\*-win_amd64.whl wheelhouse +dir wheelhouse +endlocal +exit /b + +:build_libs + set torch_version=%1 + call :check_availability + if %errorlevel%==0 ( + call :get_torch_url + mkdir %tvm_ffi%\.venv + uv venv %tvm_ffi%\.venv\torch%torch_version% --python %python_version% + call %tvm_ffi%\.venv\torch%torch_version%\Scripts\activate + uv pip install setuptools ninja + uv pip install torch==%torch_version% --index-url !torch_url! + uv pip install -v . + mkdir %tvm_ffi%\lib + python -m tvm_ffi.utils._build_optional_torch_c_dlpack --output-dir %tvm_ffi%\lib + python -m tvm_ffi.utils._build_optional_torch_c_dlpack --output-dir %tvm_ffi%\lib --build-with-cuda + call deactivate + rmdir -s -q %tvm_ffi%\.venv\torch%torch_version% Review Comment:  This build block has a couple of issues that can make the script fail: 1. The `mkdir` commands on lines 34 and 40 are not idempotent and will fail if the directories already exist on subsequent loop iterations. 2. The `lib` directory is created inside the `if` block. If no builds are performed, this directory is never created, causing the `copy` command on line 14 to fail. These can be fixed by creating the directories idempotently before they are needed. ``` set torch_version=%1 if not exist "%tvm_ffi%\lib" mkdir "%tvm_ffi%\lib" call :check_availability if %errorlevel%==0 ( call :get_torch_url if not exist "%tvm_ffi%\.venv" mkdir "%tvm_ffi%\.venv" uv venv %tvm_ffi%\.venv\torch%torch_version% --python %python_version% call %tvm_ffi%\.venv\torch%torch_version%\Scripts\activate uv pip install setuptools ninja uv pip install torch==%torch_version% --index-url !torch_url! uv pip install -v . python -m tvm_ffi.utils._build_optional_torch_c_dlpack --output-dir %tvm_ffi%\lib python -m tvm_ffi.utils._build_optional_torch_c_dlpack --output-dir %tvm_ffi%\lib --build-with-cuda call deactivate rmdir -s -q %tvm_ffi%\.venv\torch%torch_version% ``` ########## addons/torch_c_dlpack_ext/build_aot_wheels.bat: ########## @@ -0,0 +1,88 @@ +@echo off +setlocal enabledelayedexpansion + +set arch=%~1 +set python_version=%~2 + +set tvm_ffi=%cd% +set torch_c_dlpack_ext=%tvm_ffi%\addons\torch_c_dlpack_ext + +for %%P in (2.4 2.5 2.6 2.7 2.8 2.9) do ( + call :build_libs %%P +) + +copy %tvm_ffi%\lib\*.dll %torch_c_dlpack_ext%\torch_c_dlpack_ext +uv venv %tvm_ffi%\.venv\build --python %python_version% +call %tvm_ffi%\.venv\build\Scripts\activate +uv pip install build wheel auditwheel Review Comment:  The `auditwheel` package is being installed, but it is a Linux-specific tool for creating `manylinux` compliant wheels and does not work on Windows. It should be removed from this installation command. ``` uv pip install build wheel ``` ########## addons/torch_c_dlpack_ext/build_aot_wheels.bat: ########## @@ -0,0 +1,88 @@ +@echo off +setlocal enabledelayedexpansion + +set arch=%~1 +set python_version=%~2 + +set tvm_ffi=%cd% +set torch_c_dlpack_ext=%tvm_ffi%\addons\torch_c_dlpack_ext + +for %%P in (2.4 2.5 2.6 2.7 2.8 2.9) do ( + call :build_libs %%P +) + +copy %tvm_ffi%\lib\*.dll %torch_c_dlpack_ext%\torch_c_dlpack_ext +uv venv %tvm_ffi%\.venv\build --python %python_version% +call %tvm_ffi%\.venv\build\Scripts\activate +uv pip install build wheel auditwheel +cd %torch_c_dlpack_ext% +python -m build -w +dir dist +for %%f in (dist\*.whl) do python -m wheel tags "%%f" --python-tag=%python_version% --abi-tag=%python_version% --platform-tag=win_amd64 +dir dist +mkdir wheelhouse +copy dist\*-win_amd64.whl wheelhouse +dir wheelhouse +endlocal +exit /b + +:build_libs + set torch_version=%1 + call :check_availability + if %errorlevel%==0 ( + call :get_torch_url + mkdir %tvm_ffi%\.venv + uv venv %tvm_ffi%\.venv\torch%torch_version% --python %python_version% + call %tvm_ffi%\.venv\torch%torch_version%\Scripts\activate + uv pip install setuptools ninja + uv pip install torch==%torch_version% --index-url !torch_url! + uv pip install -v . + mkdir %tvm_ffi%\lib + python -m tvm_ffi.utils._build_optional_torch_c_dlpack --output-dir %tvm_ffi%\lib + python -m tvm_ffi.utils._build_optional_torch_c_dlpack --output-dir %tvm_ffi%\lib --build-with-cuda + call deactivate + rmdir -s -q %tvm_ffi%\.venv\torch%torch_version% + ) else ( + echo Skipping build for torch %torch_version% on %arch% with python %python_version% as it is not available. + ) + exit /b 0 + + +:check_availability + if %torch_version%==2.4 ( + if %python_version%==cp313 exit /b 1 + if %python_version%==cp314 exit /b 1 + exit /b 0 + ) + if %torch_version%==2.5 ( + if %python_version%==cp314 exit /b 1 + exit /b 0 + ) + if %torch_version%==2.6 ( + if %python_version%==cp314 exit /b 1 + exit /b 0 + ) + if %torch_version%==2.7 ( + if %python_version%==cp314 exit /b 1 + exit /b 0 + ) + if %torch_version%==2.8 ( + if %python_version%==cp314 exit /b 1 + exit /b 0 + ) + if %torch_version%==2.9 ( + if %python_version%==cp39 exit /b 1 + exit /b 0 + ) + echo Unknown or unsupported torch version: %torch_version% >&2 + exit /b 1 + +:get_torch_url + if %torch_version%==2.4 (set torch_url=https://download.pytorch.org/whl/cu124 && exit /b 0) + if %torch_version%==2.5 (set torch_url=https://download.pytorch.org/whl/cu124 && exit /b 0) + if %torch_version%==2.6 (set torch_url=https://download.pytorch.org/whl/cu126 && exit /b 0) + if %torch_version%==2.7 (set torch_url=https://download.pytorch.org/whl/cu128 && exit /b 0) + if %torch_version%==2.8 (set torch_url=https://download.pytorch.org/whl/cu129 && exit /b 0) + if %torch_version%==2.9 (set torch_url=https://download.pytorch.org/whl/cu129 && exit /b 0) + echo Unknown or unsupported torch version: %torch_version% >&2 + exit /b 1 Review Comment:  The series of `if` statements can be refactored to be more maintainable and less repetitive. You can determine the CUDA version string first, and then construct the URL. This is especially useful since some torch versions share the same CUDA version for the download URL. ``` set "cuda_ver=" if "%torch_version%"=="2.4" set "cuda_ver=cu124" if "%torch_version%"=="2.5" set "cuda_ver=cu124" if "%torch_version%"=="2.6" set "cuda_ver=cu126" if "%torch_version%"=="2.7" set "cuda_ver=cu128" if "%torch_version%"=="2.8" set "cuda_ver=cu129" if "%torch_version%"=="2.9" set "cuda_ver=cu129" if defined cuda_ver ( set "torch_url=https://download.pytorch.org/whl/%cuda_ver%"; exit /b 0 ) echo Unknown or unsupported torch version: %torch_version% >&2 exit /b 1 ``` -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
