Skip to content

fix(rocke): skip the dsl_docs check where the docs are not installed - #15002

Merged
cgmillette merged 3 commits into
developfrom
users/cgmillette/rocke/doc-test-installed
Nov 4, 2026
Merged

cgmillette merged 3 commits into
developfrom
users/cgmillette/rocke/doc-test-installed

Conversation

@cgmillette

@cgmillette cgmillette commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

The installed platform pytest run (rocke_pytest, run from the installed test payload) fails on develop with two FileNotFoundErrors in the fp64 GEMM doc-consistency test. The test read dtype_support.md and SUPPORT_MATRIX.md through source-tree paths:

  • dsl_docs/ is not part of the installed test payload, so dtype_support.md is not there.
  • The payload installs the rocke package beside tests/, not under python/, so platform_root() / "python" / "rocke" / ... points one level above the payload, at a path that does not exist.

The Windows test shards showed the failure. The Linux installed layout is the same and fails the same way.

Technical Details

  • Add rocke.tests.source_tree.require_dsl_docs(). It returns the dsl_docs tree, or skips the calling test with the reason when the tree is absent. The skip is keyed on the tree, not on the file: where the tree exists, a renamed or deleted doc still fails the test.
  • Split the doc check into two tests that share one assertion:
    • test_dtype_support_doc_names_every_f64_gemm_target reaches dtype_support.md through the helper. It runs from a source checkout and skips in the installed payload.
    • test_support_matrix_names_every_f64_gemm_target reads SUPPORT_MATRIX.md through the rocke.instances package. The installed payload installs the package directory whole, so this test checks the installed copy too.
  • tests/tools/test_dsl_probes.py now uses the same helper instead of its own skip. That file is not installed, so this change is for consistency only.

Test Plan

  • From a source checkout: both doc tests run and pass. With ROCKE_DSL_DOCS pointed at a copy of dtype_support.md whose summary sentence names only one fp64 GEMM target, the dtype test fails.
  • Installed payload (Linux, cmake --install of the platform in provider mode, run the way the test job runs it, PYTHONPATH=<install>/bin/hip_kernel_provider):
    • Before the fix: both doc tests fail with the same FileNotFoundErrors as CI.
    • After the fix: the dtype_support.md test skips with the reason, and the SUPPORT_MATRIX.md test passes. Narrowing a sentence in the installed SUPPORT_MATRIX.md makes that test fail.
  • Full installed platform pytest, before and after; full source platform pytest against develop; build-tree CTest; pre-commit.

Test Result

  • Source checkout: both doc tests pass, and the narrowed-doc check fails as intended.
  • Installed payload: 2 failures before, 0 after. The dtype test skips with "source-tree check: no dsl_docs tree at ...; the installed test payload does not ship it".
  • Full installed run: the only difference before and after is these two tests. The local-only portable-IR replay CLI failures appear in both runs because the run was outside CTest, which normally supplies ROCKE_REPLAY_CLI.
  • Source suite: no new failures compared with develop.
  • Build-tree CTest and pre-commit: pass.

Fixes #14998

ISSUE ID : AICK-2971

🤖 Generated with Claude Code

The fp64 GEMM doc-consistency test read dtype_support.md and
SUPPORT_MATRIX.md through source-tree paths. The installed test payload
does not carry dsl_docs/, and it places the rocke package beside the
tests rather than under python/, so both lookups pointed at files that
do not exist there and the installed platform pytest run failed.

Add rocke.tests.source_tree.require_dsl_docs(): it returns the dsl_docs
tree, or skips the calling test with the reason when the tree is absent.
The skip is keyed on the tree, so a doc missing from a present tree
still fails. The check is now two tests sharing one assertion: the
dtype_support.md test reaches the doc through the helper, and the
SUPPORT_MATRIX.md test reads it through the rocke.instances package, so
the installed payload, which installs the package directory whole,
still checks its copy. The dsl_probes test uses the same helper in
place of its own skip.

Co-Authored-By: Claude <noreply@anthropic.com>
@therock-pr-bot

therock-pr-bot Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@cgmillette
cgmillette enabled auto-merge (squash) November 3, 2026 22:58
@cgmillette

Copy link
Copy Markdown
Contributor Author

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #15002 — skip the dsl_docs check where the docs are not installed (cgmillette, AICK-2971) — 🟠 First-round: fix holds; helper untested

Prior item Status
shumway (11-03) approval 🟢 APPROVED 1d53250014
• older head; still counts on develop
• head since: a develop merge only, PR diff unchanged (ran)
Bots / CI 🟡 Running
• running: Multi-Arch builds, hipDNN Linux superbuild; 20 ✓ · 0 ✗
• CI has not run the fix: old head's hipkernelprovider shards cancelled (read)
• 1d53250014 Linux compiler-runtime ✗: runner checkout infra, no build (read)
• merge: blocked on checks; auto-merge armed (squash)
Our checks 🟢 Fix verified
• merge vs 26aa9392f5: clean; head's develop merge has no hand edits (ran)
• build + ctest 45 ✓ / 1 skip; byte identity GREEN ×4; goldens PASS (ran)
• tests 18 ✓ / 17 skip (GPU arch); compile 0 spills; rules 1–3 clean (ran)
• provider cmake --install: develop fails both doc tests, as CI (ran)
• head there: dtype test skips with its reason; SUPPORT_MATRIX ✓ (ran)
• docs present: both ✓; a narrowed doc or missing file fails (ran)
• whole installed rocke_pytest: only these 2 tests differ from develop (ran)
Code quality 🟠 Should-fix
• helper builds on dsl_docs_dir(), right test layer (read)
• nothing fails if the helper skips a present tree: N1 (ran)
• 🟡 library/tests/test_dsl_probe_attention_demos.py:35-37 keeps its own skip; use the helper (read)
# Sev file:line Finding
N1 🟠 platform/python/rocke/tests/source_tree.py:34 No test fails if require_dsl_docs() skips a present tree
• impact: an always-skip helper silently skips the dtype doc check and test_dsl_probes.py: 1 ✓ / 8 skip / 0 ✗ (ran)
• fix: platform/tests/support/test_source_tree.py: present tree returned, a skip there fails; absent tree skips, naming it (as test_gpu.py:64-66)

Utility: 🟢 Positive — unblocks installed rocke_pytest; SUPPORT_MATRIX stays checked in CI.


Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

First-round review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: merge with develop, build + ctest, byte identity, goldens, compile, doc tests in a cmake --install provider tree (docs absent and present, negative controls), whole installed rocke_pytest vs develop, CI install manifest and logs, code quality. Flags whether a closer look is needed; not an approval or rejection.

Add tests for require_dsl_docs(): it returns a present tree, so a skip
there fails, and it skips an absent tree naming the path. The attention
probe driver test now uses the helper instead of its own skip.

Co-Authored-By: Claude <noreply@anthropic.com>
@cgmillette

Copy link
Copy Markdown
Contributor Author

Re our review (#15002 (comment)):

Thanks, both items are fixed in f1139be5be8.

N1: added platform/tests/support/test_source_tree.py. It points require_dsl_docs() at a tree through its documented ROCKE_DSL_DOCS override. With the tree present the helper must return it, and a skip there fails the test. With the tree absent it must skip, and the test checks the exact message, which names the missing path. As a check, making the helper always skip fails test_returns_a_present_tree.

Minor: library/tests/test_dsl_probe_attention_demos.py now calls require_dsl_docs() instead of its own skip, so it follows the same rule: it skips where the tree is absent, and a probe missing from a present tree fails the test. test_library_layering.py still passes (tests may import any package, and library may import rocke).

@cgmillette
cgmillette requested a review from a team as a code owner November 3, 2026 23:42
@github-actions github-actions Bot added the rocke-lib: attention Touches rocKE library attention/FMHA kernels label Nov 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the change separating source-only documentation checks from packaged-resource checks.

No open issues 🎉

What changed in this PR

Fixes rocKE’s installed-test documentation failures described in #14998 by distinguishing source-only documentation from packaged resources.

Changes:

  • Adds require_dsl_docs() to skip source-only checks when the documentation tree is absent, while preserving failures for missing files within an existing tree.
  • Splits the fp64 GEMM documentation checks and reads SUPPORT_MATRIX.md through its package so it remains checked after installation.
  • Reuses the helper in probe tests and adds coverage for its return and skip behavior.
File Description
dnn-providers/​hip-kernel-provider/​rocke/​platform/​tests/​tools/​test_dsl_probes.py Uses the shared documentation-tree helper when loading probes.
dnn-providers/​hip-kernel-provider/​rocke/​platform/​tests/​support/​test_source_tree.py Tests present-tree handling and absent-tree skip messages.
dnn-providers/​hip-kernel-provider/​rocke/​platform/​tests/​numeric/​test_f64_gemm_numeric.py Separates source-only and packaged documentation checks.
dnn-providers/​hip-kernel-provider/​rocke/​platform/​python/​rocke/​tests/​source_tree.py Adds the reusable source-documentation helper.
dnn-providers/​hip-kernel-provider/​rocke/​library/​tests/​test_dsl_probe_attention_demos.py Uses the shared helper to check documentation-tree availability.

Review effort: Balanced

@cgmillette

Copy link
Copy Markdown
Contributor Author

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #15002 — skip the dsl_docs check where the docs are not installed (cgmillette, AICK-2971) — 🟡 Follow-up: both fixed; Windows shards pass

Prior item Status
Ours 11-03 N1: no test fails if helper skips a present tree 🟢 FIXED f1139be5be
• always-skip helper fails test_returns_a_present_tree (ran)
• never-skip fails the absent-tree case; saved probe FIXED (ran)
Ours 11-03 🟡: demo test keeps its own skip 🟢 FIXED f1139be5be
• present tree missing a probe: 17 ✗; develop skipped all (ran)
• absent tree: 17 skip, helper's reason; layering 4 ✓ (ran)
shumway (11-03) approval 🟢 APPROVED 1d53250014
• older head; still counts on develop
AnaghaRaoAMD (11-03) approval 🟢 APPROVED f1139be5be
• at head
Copilot (11-03) overview at head: helper, doc split, helper tests 🟢 Reviewed
• no open issues; tree-keyed skip, packaged SUPPORT_MATRIX, helper tests hold (ran)
Bots / CI 🟡 Running
• Windows hipkernelprovider ×2 ✓: rocke_pytest 0 ✗; new tests ✓, doc test skips (read)
• attempt 1: math-libs gfx94X, gfx125X ✗ — TheRock's therock-examples-cpp-sdk-user (read)
• so Linux tests skipped, required Summary ✗ (read)
• running: auto re-run of both math-libs jobs; 34 ✓
• merge: blocked until the Summary passes; auto-merge armed
Our checks 🟢 Clean
• merge vs 91a40bdfd4, 0e689f1b7f: clean (ran)
• build + ctest 45 ✓ / 1 skip; byte identity GREEN ×4; goldens PASS (ran)
• tests 37 ✓ / 17 skip (GPU arch); compile 0 spills; rules 1–3 clean (ran)
• installed rocke_pytest: new test 2 ✓, SUPPORT_MATRIX ✓; dtype doc skips (ran)
Code quality 🟢 Clean
• N1 fixed: an always-skip helper fails the new test (ran)
• the probe-driver test now calls the helper (ran)
• new test in tests/support/; rocke_pytest collects it (read)
# Sev file:line Finding
N2 🟡 PR body, Technical Details Body omits f1139be5be's two test changes
• impact: armed squash text lacks the helper's test and the demo test's switch (read)
• fix: a bullet for each; auto-merge stays armed

Utility: 🟢 Positive — unblocks installed rocke_pytest; the helper's skip is now pinned both ways.


Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: merge with develop, build + ctest, byte identity, goldens, compile, saved probe and helper mutants (source and installed layouts), the demo test with docs present, absent and missing a probe, installed rocke_pytest, CI shard logs, code quality. Flags whether a closer look is needed; not an approval or rejection.

@cgmillette
cgmillette disabled auto-merge November 4, 2026 02:59
@cgmillette
cgmillette enabled auto-merge (squash) November 4, 2026 03:00
@cgmillette
cgmillette merged commit fdd976b into develop Nov 4, 2026
154 of 157 checks passed
@cgmillette
cgmillette deleted the users/cgmillette/rocke/doc-test-installed branch November 4, 2026 03:50
AnaghaRaoAMD added a commit that referenced this pull request Nov 4, 2026
Picks up #15002, which skips the dsl_docs checks where the docs are not
installed (the hipkernelprovider rocke_pytest failures on this PR).

Co-Authored-By: Claude <noreply@anthropic.com>
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

organization: ROCm project: hip-kernel-provider rocke: platform Touches rocKE platform rocke-lib: attention Touches rocKE library attention/FMHA kernels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rocke] rocke_pytest fails in CI's installed tree since #14328: the new fp64 doc test reads docs that are not installed

4 participants