Repository navigation
[tensilelite] Re-apply test component config and fix PyTorch SDK test failure - #5529
Conversation
013446e to
86bab80
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
86bab80 to
f952643
Compare
…al build - Simplify test order description and unify usage lines - Default THEROCK_BIN_DIR to THEROCK_DIR/build/bin for local runs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
geomin12
left a comment
There was a problem hiding this comment.
thanks for fixing and confirming it works for pytorch releases. lgtm
Keep gfx1250 common GEMM test section and libexec artifact path that were added on this branch but not yet on main after PR #5529 merge. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extend test_tensilelite.py to run gfx1250 common GEMM tests under GPU emulation (e.g. FFM), building on the test component foundation from #5529. test_tensilelite.py: - Add UNIT_TEST_SKIP_FAMILIES set for arch-gated unit test skipping - Add gfx1250 common GEMM test section gated on AMDGPU_FAMILIES - Add lib/llvm/lib to LD_LIBRARY_PATH (tensilelite-client links libomp) - Pass --global-parameters=LibraryFormat='msgpack' to match prebuilt client compiled with TENSILE_MSGPACK - Scope pytest to Tensile/Tests/common to avoid rocisa singleton poisoning from unit test module-level makeIsaInfoMap() calls CMakeLists.txt: - Enable TENSILELITE_ENABLE_CLIENT on Linux only (Windows MSVC has pre-existing DataInitialization.hpp compilation issues) artifact-blas.toml: - Add libexec/hipblaslt/tensilelite/** to test component Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extend test_tensilelite.py to run gfx1250 common GEMM tests under GPU emulation (e.g. FFM), building on the test component foundation from #5529. test_tensilelite.py: - Add UNIT_TEST_SKIP_FAMILIES set for arch-gated unit test skipping - Add gfx1250 common GEMM test section gated on AMDGPU_FAMILIES - Add lib/llvm/lib to LD_LIBRARY_PATH (tensilelite-client links libomp) - Pass --global-parameters=LibraryFormat='msgpack' to match prebuilt client compiled with TENSILE_MSGPACK - Scope pytest to Tensile/Tests/common to avoid rocisa singleton poisoning from unit test module-level makeIsaInfoMap() calls CMakeLists.txt: - Enable TENSILELITE_ENABLE_CLIENT on Linux only (Windows MSVC has pre-existing DataInitialization.hpp compilation issues) artifact-blas.toml: - Add libexec/hipblaslt/tensilelite/** to test component Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| if so_path.name.endswith(".abi3.so") or ".cpython-" in so_path.name: | ||
| # Python C extensions use symbols resolved at import time, | ||
| # not via dlopen — ctypes.CDLL fails across interpreter | ||
| # versions (e.g. .abi3.so using PyType_FromMetaclass on <3.12). | ||
| continue |
There was a problem hiding this comment.
Tests with rocm[devel] installed are still failing: https://github.com/ROCm/TheRock/actions/runs/27242046092/job/80464497429?pr=5731#step:11:52
This also needs to be added to https://github.com/ROCm/TheRock/blob/main/build_tools/packaging/python/templates/rocm/src/rocm_sdk/tests/devel_test.py
This test plan was insufficient. The tests that failed were rocm python package tests, using |
## Summary Add `tensilelite` to the blas `projects_to_test` list in `therock_matrix.py` so tensilelite/rocisa tests run in TheRock CI. ## Technical details - Enable the `TensileLite` component in `therock_matrix.py`. - Guarded `from hip import hip` with try/except in `gpu_test_helpers.py` to prevent `ImportError` on systems without hip. Added `requires_gpu` marker combining hip availability + gfx950 architecture check, applied to all GPU unit test classes. (only subtile unit tests require gfx950 and `hip`) ### Dependency - #6205: Added `CMake` install rules to package `TensileLite`'s Python packages (Tensilelite + rocisa) for artifact-based testing in `TheRock` CI. - #7716: Narrowed xfail marker in `test_PlaceholderMerge.py` to only 2 tests that fail in absence of the logic files - ROCm/TheRock#5529: Added the `tensilelite` test component configuration. - #8196: Update `TheRock` reference to include ROCm/TheRock#5529 changes ## Test plan - [x] `TheRock` CI blas test job picks up `tensilelite` and runs tensilelite/rocisa tests - [x] GPU unit tests skip cleanly (not error) when `hip` module is not installed ## Test results - [x] Artifact layout verified — `share/hipblaslt/tensilelite/{Tensile,rocisa,rocisa_tests}/` - [x] Smoke import: `Tensile.ROOT_PATH` and `rocisa.__file__` resolve to artifact paths - [x] [rocisa](https://github.com/ROCm/rocm-libraries/actions/runs/27043375484/job/79836587812?pr=7872#step:11:92): 48 passed - [x] TensileLite [unit tests](https://github.com/ROCm/rocm-libraries/actions/runs/27043375484/job/79836587812?pr=7872#step:11:1569): - 1325 passed - 212 skipped (subtile tests requires [gfx950 runners](ROCm/TheRock#3288)) - 1 xfailed ([Issue](#7481) filed) AIGECORE-1997 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: therockbot <therockbot@amd.com>
Extend test_tensilelite.py to run gfx1250 common GEMM tests under GPU emulation (e.g. FFM), building on the test component foundation from #5529. test_tensilelite.py: - Add UNIT_TEST_SKIP_FAMILIES set for arch-gated unit test skipping - Add gfx1250 common GEMM test section gated on AMDGPU_FAMILIES - Add lib/llvm/lib to LD_LIBRARY_PATH (tensilelite-client links libomp) - Pass --global-parameters=LibraryFormat='msgpack' to match prebuilt client compiled with TENSILE_MSGPACK - Scope pytest to Tensile/Tests/common to avoid rocisa singleton poisoning from unit test module-level makeIsaInfoMap() calls CMakeLists.txt: - Enable TENSILELITE_ENABLE_CLIENT on Linux only (Windows MSVC has pre-existing DataInitialization.hpp compilation issues) artifact-blas.toml: - Add libexec/hipblaslt/tensilelite/** to test component Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extend test_tensilelite.py to run gfx1250 common GEMM tests under GPU emulation (e.g. FFM), building on the test component foundation from #5529. test_tensilelite.py: - Add UNIT_TEST_SKIP_FAMILIES set for arch-gated unit test skipping - Add gfx1250 common GEMM test section gated on AMDGPU_FAMILIES - Add lib/llvm/lib to LD_LIBRARY_PATH (tensilelite-client links libomp) - Pass --global-parameters=LibraryFormat='msgpack' to match prebuilt client compiled with TENSILE_MSGPACK - Scope pytest to Tensile/Tests/common to avoid rocisa singleton poisoning from unit test module-level makeIsaInfoMap() calls CMakeLists.txt: - Enable TENSILELITE_ENABLE_CLIENT on Linux only (Windows MSVC has pre-existing DataInitialization.hpp compilation issues) artifact-blas.toml: - Add libexec/hipblaslt/tensilelite/** to test component Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
## Summary Add `tensilelite` to the blas `projects_to_test` list in `therock_matrix.py` so tensilelite/rocisa tests run in TheRock CI. ## Technical details - Enable the `TensileLite` component in `therock_matrix.py`. - Guarded `from hip import hip` with try/except in `gpu_test_helpers.py` to prevent `ImportError` on systems without hip. Added `requires_gpu` marker combining hip availability + gfx950 architecture check, applied to all GPU unit test classes. (only subtile unit tests require gfx950 and `hip`) ### Dependency - #6205: Added `CMake` install rules to package `TensileLite`'s Python packages (Tensilelite + rocisa) for artifact-based testing in `TheRock` CI. - #7716: Narrowed xfail marker in `test_PlaceholderMerge.py` to only 2 tests that fail in absence of the logic files - ROCm/TheRock#5529: Added the `tensilelite` test component configuration. - #8196: Update `TheRock` reference to include ROCm/TheRock#5529 changes ## Test plan - [x] `TheRock` CI blas test job picks up `tensilelite` and runs tensilelite/rocisa tests - [x] GPU unit tests skip cleanly (not error) when `hip` module is not installed ## Test results - [x] Artifact layout verified — `share/hipblaslt/tensilelite/{Tensile,rocisa,rocisa_tests}/` - [x] Smoke import: `Tensile.ROOT_PATH` and `rocisa.__file__` resolve to artifact paths - [x] [rocisa](https://github.com/ROCm/rocm-libraries/actions/runs/27043375484/job/79836587812?pr=7872#step:11:92): 48 passed - [x] TensileLite [unit tests](https://github.com/ROCm/rocm-libraries/actions/runs/27043375484/job/79836587812?pr=7872#step:11:1569): - 1325 passed - 212 skipped (subtile tests requires [gfx950 runners](ROCm/TheRock#3288)) - 1 xfailed ([Issue](#7481) filed) AIGECORE-1997 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: therockbot <therockbot@amd.com>
…5590) ## Summary Extend TensileLite test infrastructure to support gfx1250 common GEMM tests under GPU emulation, building on the test component foundation from #5529. ## Technical details - `test_tensilelite.py`: - Add `UNIT_TEST_SKIP_FAMILIES` set for arch-gated unit test skipping (currently `gfx1250`) - Add gfx1250 common GEMM test section to `test_tensilelite.py` - Use `-m "gfx1250 or gfx12" -k gfx1250` to select tests from both `gfx1250/` directories and `gemm/gfx12/*gfx1250*` files - Honor `TEST_PROFILE` env var: `default` runs curated 57-test subset (~17 min), `nightly` runs all gfx1250 tests - Add `lib/llvm/lib` to `LD_LIBRARY_PATH` (fixes `libomp.so` load failure for `tensilelite-client`) - `FFM_QUICK_INCLUDE`: 57 curated tests with `.yaml` suffix patterns and path prefixes for disambiguation - `CMakeLists.txt`: Enable `TENSILELITE_ENABLE_CLIENT` on Linux only (Windows MSVC has pre-existing `DataInitialization.hpp` compilation issues) - Add `libexec/hipblaslt/tensilelite/**` to `artifact-blas.toml` test component ## Linked PRs - Companion rocm-libraries PR: [ROCm/rocm-libraries#8004](ROCm/rocm-libraries#8004) — client install rule + `makeIsaInfoMap` fixture fix + xfail marks for 5 known-failing streamk tests ## Test plan - [x] CI build passes (existing tensilelite tests unaffected — common test section is a no-op without gfx1250 emulator) - [x] Windows build passes (client build disabled on Windows) - [x] gfx1250 emulation run: `tensilelite-client` launches without `libomp.so` errors - [x] Quick subset selects exactly 57 tests, no ambiguous `-k` matches ## Test results - [x] 78/83 gfx1250 tests pass, 5 streamk MX tests xfail'd (solution validation regression) - [x] Quick subset: 57 tests in 16:50, timing consistent across 5 iterative runs AIHPBLAS-3508 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…est (#5756) ## Summary Skip Python C extensions (`.abi3.so`, `.cpython-*.so`) in the `devel_test.py` shared library load test. ## Technical details Python C extensions use symbols resolved at import time by the interpreter, not via `dlopen`. `ctypes.CDLL` fails when the extension uses APIs unavailable in the test interpreter version — e.g. `_rocisa.abi3.so` uses nanobind's `PyType_FromMetaclass` which requires Python 3.12+, causing `OSError: undefined symbol` on Python 3.11. This is a general fix: any `.abi3.so` or `.cpython-*.so` file should be tested via `import`, not `ctypes.CDLL`. Fixes the regression introduced by #5529 (tensilelite test artifact install with `ROCISA_USE_STABLE_ABI=ON`). ## Test plan - [ ] `test_rocm_wheels.yml` passes with `rocm[devel]` on Python 3.11 AIHPBLAS-3508 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
| joblib>=1.4.0 | ||
| numpy>=1.26.0 | ||
| filelock>=3.0.0 | ||
| hip-python; sys_platform != "win32" |
There was a problem hiding this comment.
@archana-ramalingam this now pulls in hip-python==7.2.2.562.43 which seems off as we build ROCm 7.14 and newer via TheRock. What are these outdated binding needed for? I tend to remove it from the requirements.
CC @geomin12, @ScottTodd
There was a problem hiding this comment.
This was originally added as gfx950 GPU subtile tensilelite tests require hip-python.
But since gfx950 runners aren't available on TheRock yet, this is safe to remove now.
When gfx950 runners are added back, hip-python can come from the runner setup/ROCm install.
There was a problem hiding this comment.
I don't think we have hip-python in TheRock as of today but I will send a revert anyway if this isn't needed as of today.
There was a problem hiding this comment.
We do have gfx950 runners:
TheRock/build_tools/github_actions/amdgpu_family_matrix.py
Lines 288 to 300 in ee7bf10
There was a problem hiding this comment.
gfx950 runners weren't widely available to run pre-submit tests at the time this was merged - #3288 (comment).
Hence added a TODO earlier:
@ScottTodd Either ways, is this still the case, or are they available to run pre-submits now? You can update on the issue too, if things have changed.
There was a problem hiding this comment.
Some update update PRs like https://github.com/ROCm/TheRock/actions/runs/29743540973 from #6688 add the ci:run-all-archs label which does test gfx950, and it can also be tested on-demand via an opt-in label.
There was a problem hiding this comment.
This is great, thanks for the update!
> [!IMPORTANT] > This relates to the following PRs: > * ROCm/libhipcxx#29 > * ROCm/rocm-libraries#11396 > * ROCm/rocm-libraries#11438 ## Motivation * Fixes #7491 We've been using a common https://github.com/ROCm/TheRock/blob/main/requirements-test.txt file across all ROCm subproject tests that seemed convenient to extend at the time, but it has a few drawbacks: * #5529 added 50MB+ of dependencies only used for tensilelite tests but downloaded for all subprojects, sometimes contributing up to 50 minutes of job time _per CI run_ just downloading packages on runners with slow network and no pip cache * #6106 required sending a pull request to TheRock in order to run "TensileLite characterization (snapshot) tests", when such changes could be made purely here in rocm-libraries if we keep the responsibility for maintaining requirements lists next to the test code itself ## Technical Details > [!IMPORTANT] > Now each project that installs additional requirements should provide its own `requirements.txt` as part of its test artifact and register that with the CI test system via the `additional_requirements_files` field in `build_tools/github_actions/fetch_test_configurations.py`. Comparing https://github.com/ROCm/TheRock/actions/runs/33191445520 (this branch) vs a baseline https://github.com/ROCm/TheRock/actions/runs/33159247177, in aggregate across up to 50 subproject test jobs, this results in: * Most test jobs now download ~15MB instead of 110MB of python packages --> save ~4.5GB of download bandwidth per full CI run * Installing requirements median time dropped from 39s to 2.5s --> save ~24 runner-minutes per full CI run ## Test Plan * Tested via `workflow_dispatch`: https://github.com/ROCm/TheRock/actions/runs/33191445520 and checked that jobs complete successfully after installing the expected set of packages * Note the 'quick' test levels, while projects run additional tests that other test levels that _may_ have additional requirements ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md.
## Motivation * Progress on ROCm/TheRock#7491 We've been using a common https://github.com/ROCm/TheRock/blob/main/requirements-test.txt file across all ROCm subproject tests that seemed convenient to extend at the time, but it has a few drawbacks: * ROCm/TheRock#5529 added 50MB+ of dependencies only used for tensilelite tests but downloaded for all subprojects, sometimes contributing up to 50 minutes of job time _per CI run_ just downloading packages on runners with slow network and no pip cache * ROCm/TheRock#6106 required sending a pull request to TheRock in order to run "TensileLite characterization (snapshot) tests", when such changes could be made purely here in rocm-libraries if we keep the responsibility for maintaining requirements lists next to the test code itself Including `requirements-test.txt` in the test artifact will also allow users installing the test artifacts/tarballs to know which packages are needed too. ## Technical Details The contents of this `projects/hipblaslt/tensilelite/requirements-test.txt` file were lifted from both https://github.com/ROCm/TheRock/blob/main/requirements-test.txt and an analysis of tensilelite/rocisa test entry points. I'm not sure if this is the exact list that these projects want, but once the project is in direct control of the requirements further edits can be made more easily. ## Test Plan I'm testing this together with some changes in TheRock that will start picking up this requirements file from the test artifact. See https://github.com/ROCm/TheRock/actions/runs/33191445520/job/98940712172. It should be safe to add the requirements here and then start using them from TheRock later. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Tony Davis <tony.davis@amd.com> Co-authored-by: T.J. Alumbaugh <talumbau@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Summary
Re-apply tensilelite test component configuration (reverted in #5520) and fix the
testSharedLibrariesLoadfailure on Python 3.10/3.11.Technical details
_rocisa.abi3.so: undefined symbol: PyType_FromMetaclasson Python 3.10/3.11 #5516:_rocisa.abi3.sousesPyType_FromMetaclass(Python 3.12+ symbol).testSharedLibrariesLoadtest loads all.sofiles viactypes.CDLL, which fails for Python C extensions under older interpreters..abi3.soand.cpython-*extensions inlibraries_test.py— these must be loaded viaimportlib, notctypes.CDLL. Same pattern as the existingamd_smi/goamdsmiskip.-DROCISA_USE_STABLE_ABI=ONis preserved — the stable ABI build is correct; the issue was only in how the PyTorch SDK test loads the.so.Test plan
testSharedLibrariesLoadpasses on Python 3.10/3.11 (no longer attempts to ctypes-load_rocisa.abi3.so)test_tensilelite.py:Fixes #5516
AIGECORE-1997