[hipDNN] ALMIOPEN-1943 Add pytest suite and CI workflow for Python bindings#7600
Draft
tvy-amd wants to merge 113 commits into
Draft
[hipDNN] ALMIOPEN-1943 Add pytest suite and CI workflow for Python bindings#7600tvy-amd wants to merge 113 commits into
tvy-amd wants to merge 113 commits into
Conversation
…arking changes Rewrite python/CMakeLists.txt to use scikit-build-core's SKBUILD variable for wheel-specific install rules (following the stinkytofu pattern) instead of unconditional dist-packages install. Regular cmake --install stages the wheel via pip wheel in an else() branch, avoiding recursion. Revert all dnn-benchmarking packaging changes to keep this branch focused on hipdnn-frontend Python bindings only. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Let scikit-build-core handle Python files via wheel.packages instead of cmake install rules. Simplifies SKBUILD block to only install the .so. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Installs the built hipdnn-frontend wheel into a fresh venv and runs 17 smoke tests covering imports, enums, Tensor/Graph construction, method chaining, serialization helpers, and engine ID management. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
When built via TheRock superbuild, nanobind is provided as a third-party dependency. For standalone builds, FetchContent fetches it from GitHub. Also bumps FetchContent tag from v1.8.0 to v2.4.0. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
…ndings Replace inline FetchContent for nanobind with the project's hipdnn_add_dependency pattern so find-or-fetch behavior is consistent with all other dependencies. Add tsl-robin-map as a separate dependency since nanobind is configured with NB_USE_SUBMODULE_DEPS OFF. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Exclude Python, nanobind, and _deps headers from clang-tidy's header filter to prevent false positives on third-party code. Rename binding functions from snake_case to camelBack to match project naming conventions, and add braces around bare if/else bodies. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Add shared bindings.hpp header with forward declarations to resolve misc-use-internal-linkage. Suppress unavoidable performance-no-int-to-ptr for Python FFI pointer casts. Use const references for nb::object and nb::bytes parameters. Add const to status/error variables and use auto with reinterpret_cast. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Only the module development component is needed for nanobind extension modules, not the full Development (which also requires Development.Embed and libpython). This matches the pattern used by other Python bindings in the repo (stinkytofu, origami, rocisa). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
…ding package locations When pip wheel re-invokes CMake on python/CMakeLists.txt standalone, hipdnn_add_dependency is not available. Include Dependencies.cmake as a fallback. Forward nanobind_DIR, tsl-robin-map_DIR, and version variables so find_package() picks up already-built packages from TheRock without re-downloading. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The version variables are unnecessary — Dependencies.cmake has matching hardcoded defaults, and when packages are found via nanobind_DIR / tsl-robin-map_DIR the version is not used. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Add integration tests for conv_fprop, conv_dgrad, conv_wgrad, and matmul that mirror the existing C++ samples. Add unit-style API tests for Tensor, Graph, and DeviceBuffer. Convert the existing engine_id_overloads test to pytest format. Configure pytest in pyproject.toml with gpu and integration markers. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
…hardcoded compiler paths Forward FETCHCONTENT_SOURCE_DIR for nanobind and tsl-robin-map so the standalone wheel build reuses already-downloaded sources when the parent used FetchContent (where _DIR variables are empty). Remove hardcoded /opt/rocm/llvm/bin/clang compiler paths from pyproject.toml — let the environment or -C flags control the compiler. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The pip wheel subprocess could not find the HIP package because hip_DIR was not forwarded from the parent CMake configuration. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
The pip wheel subprocess finds HIP via hip_DIR but hip-config-amd.cmake calls find_dependency for AMDDeviceLibs, amd_comgr, and hsa-runtime64. Without forwarding these _DIR variables the isolated build environment cannot locate them, causing CMake configuration failure in TheRock CI. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
…nctions Replace _determine_git_tag indirection with direct use of the VERSION parameter for the git tag. The version is already passed from the caller via hipdnn_add_dependency so the macro lookup is unnecessary. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Replace the scikit-build-core + pip subprocess approach with a standalone wheel assembly script, eliminating the need to forward CMake dependency variables (hip_DIR, AMDDeviceLibs_DIR, etc.) through pip config settings. The superbuild path now stages the compiled .so and calls assemble_wheel.py (stdlib only) to produce the wheel directly. The developer path uses hatchling with a custom build hook that either compiles via CMake or packages a pre-built .so via HIPDNN_PREBUILT_SO. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reverts hipblas and miopen cpp/hpp diffs vs PR merge-base. Patch saved in WIP/worktrees/rocm-libraries/python_binding_tests/cpphpp-pr-diffs-2.patch for later reapply. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reverts the codegen template; underlying generated .cpp files were already reverted to PR merge-base in prior commit. Patch saved to WIP/worktrees/rocm-libraries/python_binding_tests/descriptor-j2.patch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Temporary CI hash bump for testing. Revert before merge. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…02fa9d9fe4d9ed Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (77.83%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #7600 +/- ##
===========================================
- Coverage 62.06% 62.06% -0.00%
===========================================
Files 2085 2085
Lines 357573 357573
Branches 54060 54060
===========================================
- Hits 221914 221911 -3
- Misses 116871 116874 +3
Partials 18788 18788
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
…on bindings test The hipdnn_python_bindings test script in TheRock expects rocm-libraries at external-sources/rocm-libraries/ (treated as a submodule of TheRock). Update the test-component workflow checkout path and the notify_teams script reference to match the expected layout.
Reverts the external-sources/ workflow checkout path added in da6cf75 and bumps the TheRock pin to pick up the script-side fix (TheRock@649855473) that resolves the hipDNN pytest dir via HIPDNN_PYTHON_TESTS_DIR / ROCM_LIBRARIES_DIR / sibling rocm-libraries checkout instead of the hard-coded external-sources/ submodule path.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| import numpy as np | ||
| import pytest | ||
|
|
||
| import hipdnn_frontend as hipdnn |
Contributor
Author
There was a problem hiding this comment.
Removed because helpers call hipdnn_frontend for us.
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Stage hipdnn_frontend/test/*.py into share/hipdnn/tests/python so downstream packagers (e.g. TheRock) can include them in the hipDNN test artifact. Keeps tests outside the package payload to avoid accidentally testing the staged tree instead of the installed wheel. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…inding_tests # Conflicts: # .github/workflows/therock-ci-linux.yml # .github/workflows/therock-ci-nightly.yml # .github/workflows/therock-ci-windows.yml # .github/workflows/therock-ci.yml # .github/workflows/therock-test-component.yml # .github/workflows/therock-test-packages.yml
TheRock CI uses its own pack_frontend_wheel.py for wheel packaging. The local CMake build only stages files for TheRock to consume and never invokes assemble_wheel.py.
2 tasks
…python" This reverts commit e8f6fb4.
Un-reverts the install rule the TheRock test runner depends on. test_hipdnn_frontend_python.py expects the pytest directory at build/share/hipdnn/tests/python; without it the test job fails with FileNotFoundError. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Point all TheRock CI pins to 9c0ca0a4a, which fixes the frontend wheel-pack script to use `uv build --wheel` instead of the invalid `uv pip wheel`. Also strip trailing whitespace on the ref line that failed the pre-commit trailing-whitespace hook. 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
2 participants
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
Add a pytest test suite (42 tests) for the hipDNN Python bindings and a GitHub Actions workflow to run them on GPU hardware with miopen-provider. Default
HIPDNN_BUILD_PYTHON_BINDINGSto OFF so existing builds are unaffected.Risk Assessment
Low risk. Adds new test infrastructure and a CI workflow without modifying production library code. The only product-code change is flipping a CMake option default from ON to OFF, which is additive — existing consumers who set the flag explicitly are unaffected.
Testing Summary
Testing Checklist
pytest projects/hipdnn/python/hipdnn_frontend/test/ -v- ASICs: gfx942 - Status: Passed (42/42)Technical Changes
projects/hipdnn/python/hipdnn_frontend/test/with tests covering tensor API, graph API, convolution (fprop/dgrad/wgrad), matmul, and device buffer operations.build_conv_fprop_graph,execute_graph) intohelpers.pysince pytest does not support importing fromconftest.pyas a regular module.Tensor.create()no longer auto-assigns UIDs,Graph.tensor(attrs)removed.gpu,integration) inpyproject.toml..github/workflows/hipdnn-python-tests.ymlthat builds hipDNN + miopen-provider onazure-linux-scale-rocmand runs the full suite.HIPDNN_BUILD_PYTHON_BINDINGSto OFF inCMakeLists.txt; CI workflow explicitly enables it.