Repository navigation
fix(rocke): restore TF32 tests in installed artifacts - #13195
Merged
Merged
Conversation
Install the native recipe replay CLI in the test artifact and compare CBOR replay with Python LLVM lowering without attempting a source build. Run gfx942 numerics with Python when the native binding is absent and retain strict backend parity when it is available.
github-actions
Bot
added
documentation
project: hip-kernel-provider
rocke: platform
tenpercent
assigned
brockhargreaves-amd and
tenpercent and unassigned
brockhargreaves-amd
tenpercent
marked this pull request as draft
October 6, 2026 19:26
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes consistently restore installed TF32 coverage with explicit failure handling and focused regression tests.
Review effort: Balanced
Findings: None
What changed in this PR
Restores installed-artifact TF32 testing by packaging native recipe replay support and selecting available numerical backends.
Changes:
- Installs and discovers the portable-IR replay CLI.
- Re-enables TF32 recipe and gfx942 numerical tests.
- Adds host regression coverage and documents installed-test behavior.
| File | Description |
|---|---|
TESTING.md |
Documents TF32 artifact and backend contracts. |
platform/tests/numeric/test_gfx942_tf32_numeric.py |
Selects Python-only or dual-backend validation. |
platform/tests/core/test_tf32.py |
Re-enables native CBOR replay parity tests. |
platform/tests/core/test_tf32_artifact_support.py |
Tests discovery, selection, and failure behavior. |
platform/tests/conftest.py |
Adds replay CLI discovery fixture. |
platform/tests/CMakeLists.txt |
Installs the replay CLI with test artifacts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Keep the fixture local to the TF32 module and supply its executable through a dedicated CTest environment setting. Remove test-layout discovery without changing the replay settings or enrollment of other portable-IR tests.
tenpercent
marked this pull request as ready for review
October 6, 2026 20:08
cgmillette
approved these changes
Oct 7, 2026
cgmillette
left a comment
cgmillette
left a comment
Contributor
There was a problem hiding this comment.
PR #13195 — restore TF32 tests in installed artifacts (tenpercent, AICK-2514) — 🟢 ACCEPT: approve; restored lanes are installed, collected and green in CI
| Prior item | Status |
|---|---|
Human reviews / cgmillette |
🟢 None (0 threads, 0 human reviews) |
Copilot ×2 (f19f8d0f, 7031c7c8): approve, 0 findings |
🟢 N/A: nothing raised. The shared conftest.py fixture + wrapper test it summarized were dropped (5c9715d8, 10477dcb) |
| Bots: policy check + CI | 🟢 Policy pass (AICK-2514). Multi-Arch CI @7031c7c8: installed rocke_pytest ran 10/10 TF32 replay + 4/4 portable-IR CLI PASSED (gfx942, Windows); gfx942 numeric PASSED without the binding, closing the body's deferred item. Single-arch TheRock + Math CI skipped |
| # | Sev | file:line | Net-new finding |
|---|---|---|---|
| N1 | 🟡 | test_tf32.py:389-404 |
check=True + capture_output=True discards the CLI's replay_cli: … failed: <err> stderr; a CI failure shows only the command and exit code (repro'd with a failing stand-in CLI). Assert returncode == 0, stderr[-2000:] as test_portable_ir.py:202 does. |
| N2 | 🟡 | TESTING.md:529-546, test_portable_ir.py:73-86 |
Doc/body overclaim: new §5.1.3 calls ROCKE_REPLAY_CLI "the single launcher setting" and bans cache/build-dir fallbacks; body says a missing CLI skips "matching the existing portable-IR tests". _replay_cli() instead ignores a missing path and silently runs a temp-dir or platform/build CLI (repro'd). Drop those fallbacks or reword; §5.1.2 (:494) still lists only PYTHONPATH=.. |
Utility: 🟢 Positive — swaps #12718's blanket skips for a source-build-free, relocatable native-vs-Python IR byte-identity lane (10 cases × LLVM 20/22/23), restores gfx942 numerics without the binding, and enables 4 previously-skipped portable-IR CLI tests; 1 install rule + 1 env var, captured by TheRock's existing tests/** glob. Merge; N1/N2 as follow-ups.
Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good
Contributor
TheRock Submodule Bump ActivityNewest first
|
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.
The installed rocKE suite failed eleven TF32 cases introduced by #12612: ten recipe cases attempted a native source build from the installed
bindirectory, and the gfx942 numerical test requestedrocke_engine, which the artifact does not contain. #12718 temporarily skipped these tests in every environment.Restore the tests by installing the existing native recipe replay CLI under
tests/portable_ir/, which TheRock already captures in the test artifact. Compare native CBOR replay with Python LLVM lowering for both atom shapes, all preparation modes, and LLVM 20/22/23. Installed CTest supplies a relocatable executable path through the existingROCKE_REPLAY_CLIsetting for all native CLI replay tests. TF32 keeps its module-local fixture and uses the same setting. This also enables the four existing portable-IR CLI tests in installed artifacts. An unset or missing replay executable skips the native lane, matching the existing portable-IR tests; execution errors and IR mismatches remain failures.Run all 12 Python numerical variants against the independent NumPy references when the native binding is absent. When the binding is available, require all 24 variants, native builder/lowerer byte identity, and backend numerical parity; a broken binding remains a failure. The standalone numerical CLI retains its strict both-backend default. Document general native replay rules in
TESTING.md: use prebuilt artifacts, skip absent CLIs, avoid implicit source builds, isolate fixtures, and validate relocated installs.Fixes #12718
ISSUE ID : AICK-2514
Test plan
rocke_pytestwith the shared replay setting: 1,727 passed, 321 skipped, 1,632 subtests passed.--no-deps --reinstall; CTest discovery confirmed the selected entry and installed import root.The local installed replay used the platform's provider-style installation and generated provider CTest entries, rather than the complete provider superbuild. Tools were Python 3.12.12, NumPy 2.5.3, pytest 9.1.1, pytest-timeout 2.4.0, ml-dtypes 0.6.0, and ROCm 7.1 Clang 20; these differ from the original CI runtime. Kernel emitters and numerical tolerances are unchanged.
An initial full run used
/usr/bin/llcand failed four unrelated debug-info object tests because that compiler does not recognize gfx950. The reported passing run selects ROCm LLVM viaROCKE_TEST_LLVM_BINandPATH.