[hipDNN] ALMIOPEN-1869 Enable hipDNN Python bindings build, wheel validation test, and CI matrix entry#5429
Open
tvy-amd wants to merge 51 commits into
Open
[hipDNN] ALMIOPEN-1869 Enable hipDNN Python bindings build, wheel validation test, and CI matrix entry#5429tvy-amd wants to merge 51 commits into
tvy-amd wants to merge 51 commits into
Conversation
Add PRE_ARTIFACT_COMMANDS parameter to therock_provide_artifact to allow running commands after staging but before artifact population. Use it to pack the hipDNN python bindings into a wheel via the new build_tools/pack_python_wheel.py script. - Add pack_python_wheel.py: generic wheel packer from pre-built files - Add PRE_ARTIFACT_COMMANDS to therock_artifacts.cmake - Enable HIPDNN_BUILD_PYTHON_BINDINGS=ON in hipDNN subproject - Pack wheel from staged python bindings before artifact population - Exclude staged python files from lib artifact (wheel replaces them) Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Remove PRE_ARTIFACT_COMMANDS from the hipDNN artifact declaration and let the raw Python package flow into the lib artifact instead. The wheel is now packed by build_python_packages.py alongside the other ROCm SDK wheels, using a new _pack_standalone_wheels() function that scans artifacts for pre-built Python extension modules. Also add --name and --version CLI args to pack_python_wheel.py as an alternative to --pyproject-toml so the post-build script can supply name/version directly. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
No callers remain after moving hipDNN wheel packing to the post-build Python packaging step. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Replace the generic _pack_standalone_wheels with a straightforward _pack_hipdnn_wheel that handles just the hipDNN case directly. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Make --name and --version required args. The pyproject.toml parsing and tomllib dependency are no longer needed since the caller supplies name and version directly. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The previous regex produced raw extension suffix segments like cpython-312-x86_64-linux-gnu instead of standard wheel tags. Parse Linux and Windows extension suffixes explicitly to emit correct tags (e.g. cp312-cp312-linux_x86_64). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Remove _pack_hipdnn_wheel from build_python_packages.py and move pack_python_wheel.py to test_executable_scripts. The wheel is not yet ready for production packaging — instead, capture the Python bindings in the test artifact and validate the wheel pipeline with a dedicated test script. - Revert build_python_packages.py to upstream (no hipDNN changes) - Move pack_python_wheel.py to test_executable_scripts/ - Add share/hipdnn/python/** to test component in artifact-hipdnn.toml - Add test_install_hipdnn_python_bindings.py: builds wheel, installs in temp venv, validates import, runs upstream pytests Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Points to ROCm/rocm-libraries#7352 (6ef9fd87) which adds HIPDNN_BUILD_PYTHON_BINDINGS support and installs the hipdnn_frontend package to share/hipdnn/python/. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
- Walk subdirs for extension module detection instead of glob top-level - Use pathlib for file reads, drop glob import - Add Requires-Python>=3.9 to wheel metadata - Pin pytest version range, install wheel with --no-deps - Fail with exit code 2 when pytest suite directory is missing Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
hipDNN's python/CMakeLists.txt calls find_package(tsl-robin-map) which is provided by the super-project. TheRock's strict dep provider requires it to be explicitly declared even though it is a transitive dependency of therock-nanobind. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The hipDNN Python extension fails to load in wheel installs because
libhipdnn_backend.so is not on LD_LIBRARY_PATH. The updated submodule
adds rocm_sdk.preload_libraries("hipdnn") to hipdnn_frontend/__init__.py
so the backend is loaded with RTLD_GLOBAL before the extension imports.
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The testSharedLibrariesLoad tests load .so files via ctypes.CDLL directly, bypassing Python's import mechanism. Add 'hipdnn' to the preload list so libhipdnn_backend.so is available when hipdnn_frontend_python.so is tested. Also add the preload to devel_test.py which had no preloading at all. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
…on-wheel # Conflicts: # rocm-libraries
tvy-amd
commented
May 25, 2026
Previous submodule commit 3caa7e9f2b was a local-only merge that didn't exist on the rocm-libraries remote, causing CI fetch failures. Point to bf374f7a05 which is on users/tvy/python_bindings. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
…on-wheel # Conflicts: # rocm-libraries
- pack_python_wheel.py: switch to pathlib, add type hints, deterministic file ordering, fail fast when no .so/.pyd is found (no silent py3-none-any fallback for a Root-Is-Purelib: false package), ZIP_STORED for native extensions, versioned Generator string, and optional --requires-python/--summary/--homepage/--author/--license METADATA flags. - test_install_hipdnn_python_bindings.py: pass Summary, Author, License through to the packer; add -> None hints on install_wheel and validate_import.
Replace the hand-rolled zip/PEP-427 writer with a small driver that stages the prebuilt package under generated pyproject.toml + setup.py templates and delegates to `pip wheel`. Standard tooling now owns wheel naming, METADATA assembly, RECORD generation, and tag negotiation. - pack_python_wheel.py: parse CLI, render the two templates via string.Template, copy the package directory into a tempdir, and run `pip wheel --no-deps`. - pack_python_wheel_pyproject.toml.in: build-system pinned to setuptools>=68 + wheel; [project] table holds name/version/python pin; optional Summary/Author/License/Homepage rendered into a single substitution slot when the matching CLI flag is provided. - pack_python_wheel_setup.py.in: BinaryDistribution + bdist_wheel subclass that forces Root-Is-Purelib: false and emits a cpXY-cpXY-<platform> tag derived from sys.version_info and sysconfig.get_platform(); find_packages picks up nested subpackages and package_data ships everything under the package root. Output is byte-identical in spirit to the previous packer: hipdnn_frontend-<ver>-cp3X-cp3X-linux_x86_64.whl with the same METADATA fields, but produced by setuptools/wheel rather than custom zip code.
…lags Metadata (name, description, license, author, requires-python, version) is now hardcoded in pack_python_wheel_pyproject.toml. The packer driver only takes --pkg-dir and --wheel-dir; caller no longer passes --name/--version/--summary/--author/--license. Adds .so/.pyd pre-check and pkg_name identifier validation, and simplifies package_data glob.
Package name is always hipdnn_frontend, so the ${pkg_name} placeholder
was the only dynamic piece. Hardcode it, drop the .in suffix, and have
the packer shutil.copy both pyproject.toml and setup.py into the staging
dir. Validates --pkg-dir basename matches the expected package name.
- pack_python_wheel_setup.py: drop PrebuiltWheel(bdist_wheel) subclass,
switch to plain setup() with Distribution.has_ext_modules()->True and
options={'bdist_wheel': {'plat_name': ...}}, matching the rocm-sdk-core
/ rocm-sdk-libraries template style. Add SPDX header. Exclude
__pycache__/*.pyc/*.pyo from package_data.
- pack_python_wheel_pyproject.toml: build-system requires setuptools>=70.2.0
(matches sibling templates), drop unneeded wheel pkg.
- pack_python_wheel.py: short-circuit native-extension scan via next().
- test_install_hipdnn_python_bindings.py: remove dead ArgumentParser
block (no args left after --version removal).
- artifact-hipdnn.toml: constrain share/hipdnn/python/** to *.py and *.so
to avoid shipping stray cache/build artifacts.
- devel_test.py: split inline preload command into a multi-line "; ".join
for readability.
- validate_import: run from neutral cwd (tmp_path) so import cannot resolve to a sibling staged hipdnn_frontend/ directory and yield a false positive. - artifact-hipdnn.toml: include share/hipdnn/python/**/*.pyd so Windows builds keep the hipdnn_frontend_python.pyd extension in the artifact. - pack_python_wheel.py: switch final print to logging.info for output consistency, and fail loudly if pip wheel produces no wheel file. - test_install_hipdnn_python_bindings.py: clarify Windows PATH vs Linux LD_LIBRARY_PATH asymmetry with a comment.
The test script already handles Windows (PATH vs LD_LIBRARY_PATH, .exe venv suffix), so extend the matrix entry to dispatch on Windows runners as well. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The pytest suite exercises conv, matmul, and graph_api which load engines via libmiopen_plugin.so. That plugin ships in the miopenprovider artifact, not hipdnn, so the previous `--hipdnn --tests` fetch left engines unavailable at test time. Match the miopenprovider sibling's artifact set (minus integration tests). Also fix HIPDNN_PYTHON_TESTS_DIR: the upstream pytest directory is `python/hipdnn_frontend/test`, not `python/tests`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
share/hipdnn/python/** is staged for the test component (which packages the wheel). Without an explicit exclude, the lib component also claimed those files, pulling hipdnn_frontend_python.cpython-*.so into the rocm-sdk-libraries wheel where the shared-library load test would try to dlopen it standalone and fail (the nanobind extension depends on libhipdnn_backend). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The toml exclude of share/hipdnn/python/** from the lib component removes the frontend nanobind extension from the libraries wheel, so the preload workaround is no longer needed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The script previously hard-coded THEROCK_DIR/external-sources/rocm-libraries/projects/hipdnn/... which only works when rocm-libraries is checked out as a TheRock submodule. The rocm-libraries CI workflows check out rocm-libraries as a sibling of the TheRock checkout instead. Resolve the pytest dir via HIPDNN_PYTHON_TESTS_DIR (explicit override) or ROCM_LIBRARIES_DIR (rocm-libraries root), defaulting to THEROCK_DIR/rocm-libraries/projects/hipdnn/python/hipdnn_frontend/test.
`pack_python_wheel.py` invokes `python -m pip wheel ...` against the test venv created by `setup_venv.py --use-uv`. `uv venv` does not seed pip, so the wheel build fails with `No module named pip`. Add pip to the test requirements install so the wheel-packaging path works.
The hipdnn_frontend wheel was being built by invoking `sys.executable -m pip wheel` against the venv created by `setup_venv.py --use-uv`. uv-created venvs skip ensurepip, so the venv has no pip module and the call fails with `No module named pip`. Swap to `uv pip wheel --python <interp>` so the wheel build no longer depends on pip being present in the surrounding venv. Revert the `setup_test_environment` action change that worked around this by seeding pip into the venv.
Rename test_install_hipdnn_python_bindings.py to test_hipdnn_python.py since it runs the tests; wheel build is just a prereq step. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
FYI I'm running the tests in this PR ROCm/rocm-libraries#7600 |
Default the hipDNN Python pytest dir to share/hipdnn/tests/python inside OUTPUT_ARTIFACTS_DIR so the test job validates exactly the artifacts it downloaded, matching the pattern used by rocr-debug-agent, rocgdb, libhipcxx, and rocprofiler test jobs. - artifact-hipdnn.toml: include share/hipdnn/tests/python/**/*.py in the test component. - test_hipdnn_python.py: prefer the artifact path; keep HIPDNN_PYTHON_TESTS_DIR / ROCM_LIBRARIES_DIR as developer fallbacks for local runs against a rocm-libraries checkout. Avoids initializing the rocm-libraries submodule in the hipDNN Python test job and prevents version mismatch between the wheel artifact and tests from whatever submodule SHA the TheRock checkout has. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move the wheel-packing scripts into a hipdnn/ subfolder under test_executable_scripts/ and rename them so the names are scoped to hipDNN rather than generic 'pack_python_wheel'. Also rename the test driver script for consistency. pack_python_wheel.py -> hipdnn/pack_frontend_wheel.py pack_python_wheel_setup.py -> hipdnn/pack_frontend_wheel_setup.py pack_python_wheel_pyproject.toml -> hipdnn/pack_frontend_wheel_pyproject.toml test_hipdnn_python.py -> test_hipdnn_frontend_python.py Update the corresponding references in fetch_test_configurations.py and the in-script PYPROJECT_FILE / SETUP_FILE / PACK_WHEEL_SCRIPT constants.
SamuelReeder
approved these changes
May 28, 2026
Contributor
SamuelReeder
left a comment
There was a problem hiding this comment.
LGTM! Just one question.
The ROCM_LIBRARIES_DIR env var was never set anywhere in CI nor any documented dev flow, so the branch resolving the upstream pytest dir through it was dead code. Local devs can still point at an out-of-tree test dir via HIPDNN_PYTHON_TESTS_DIR.
… share/hipdnn prefix The HIPDNN_PYTHON_TESTS_DIR escape hatch is never set in CI nor any documented dev flow, so the resolver branch and its wrapping helper were dead code. Inline the artifact-relative path at the single use site. Also factor the repeated `share/hipdnn` prefix into a module-level constant and reuse it for both the tests dir and the package dir, so adding a third consumer does not require yet another string-form Path.
`tests_dir` and the former `find_pkg_dir` both resolved a relpath under the artifact tree, checked the directory exists, and raised FileNotFoundError with a hint on miss. Factor that pattern into `_require_artifact_dir(artifacts_path, relpath, label, hint)` and call it twice from main, dropping the bespoke per-path helper.
Replace subprocess.check_call with subprocess.run(..., check=True, timeout=...) for every step the wheel test driver shells out: wheel build, venv create, pip installs, import smoke check, pytest. Without per-step timeouts a hung GPU or deadlocked pytest blocks until the CI matrix budget (30 min) expires; bounded steps fail the right step instead. Also fail fast in pack_frontend_wheel.py when 'uv' is not on PATH so the error tells the operator how to fix it instead of surfacing a generic FileNotFoundError from the subprocess call.
- Extract `build_rocm_loader_env` into `libhipcxx_utils.py` so the platform
loader-path branch (PATH on Windows, LD_LIBRARY_PATH on Linux) is shared
with future test scripts that need to load ROCm shared libs.
- Replace hand-rolled venv creation with `setup_venv.create_venv` +
`find_venv_python_exe` from `build_tools/setup_venv.py`, removing the
duplicate Windows/Linux python-exe discovery and the per-call venv timeout
(venv create is fast, CPU+IO only, doesn't touch the GPU).
- Rewrite `pack_frontend_wheel_setup.py` docstring to accurately describe
how it differs from rocm-sdk-* templates: those ship loose shared libs
and produce `py3-none-<plat>` wheels; this packages a CPython-ABI-bound
nanobind extension and needs the ABI-locked `cp{X}{Y}-cp{X}{Y}-<plat>`
tag, which `Distribution.has_ext_modules() -> True` forces.
pytest collection runs `import hipdnn_frontend` before executing any test, so an explicit one-liner import check before `run_pytests` exercises the same code path with the same loader env. Removing it drops a step (and its 60s timeout constant) without losing coverage.
BrianHarrisonAMD
approved these changes
May 28, 2026
Contributor
BrianHarrisonAMD
left a comment
There was a problem hiding this comment.
LGTM assuming CI passes in the various environments.
`uv pip wheel` is not a valid subcommand and fails with "error: unrecognized subcommand 'wheel'". Switch to `uv build --wheel` with `--out-dir`, which is the correct uv invocation for producing a wheel from a source tree. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables hipDNN Python bindings+ CI wheel test in TheRock.
Changes
Build / packaging
pack_frontend_wheel.pystages pre-builthipdnn_frontend/package next to templatepyproject.toml+setup.py, delegates touv pip wheel(uv-created venvs skip ensurepip).pack_frontend_wheel_setup.pyforcesDistribution.has_ext_modules() -> Trueso bdist_wheel tags the wheelcp{X}{Y}-cp{X}{Y}-<plat>(nanobind.sois ABI-locked, unlikepy3-none-<plat>rocm-sdk-* templates).Artifact split
share/hipdnn/python/**excluded from lib component (Python tree only ships via wheel, not the shared-library tarball).CI test
test_hipdnn_frontend_python.py: builds wheel → creates venv → installs wheel + pytest → runs upstreamshare/hipdnn/tests/pythonsuite.setup_venv.create_venv/find_venv_python_exe(no hand-rolled venv).build_rocm_loader_env(artifacts_path)helper inlibhipcxx_utils.py(Linux:LD_LIBRARY_PATH; Windows:PATH).CI matrix