Skip to content

test(rocke): check the representative-IR golden one LLVM flavor per test - #14820

Merged
aosewski merged 1 commit into
developfrom
users/cgmillette/rocke/golden-test-per-flavor
Nov 2, 2026
Merged

aosewski merged 1 commit into
developfrom
users/cgmillette/rocke/golden-test-per-flavor

Conversation

@cgmillette

@cgmillette cgmillette commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

tests/test_rocke_ci_static.py::TestIrParityCoverage::test_ir_cases_match_golden_sha256 lowers and hashes every representative-IR case at every LLVM flavor (llvm20, llvm22, llvm23, llvm24) inside a single test. In the installed rocke_pytest CTest lane, which runs pytest with --timeout=60, that one test runs close to the per-test limit and times out intermittently on develop and on unrelated PRs.

ISSUE ID : AICK-2895

Tracking issue: #14733

Technical Details

  • The golden check is now one test per LLVM flavor, test_ir_cases_match_golden_sha256[<flavor>], parametrized over GOLDEN_FLAVORS (which is LLVM_FLAVORS, the list of qualified compiler lanes), so a newly added flavor gets its own test with no edit here. Each test lowers a quarter of the work the old test did. A drift message names its flavor.
  • The checks the old all-flavor call made on the golden itself (a document for each emission profile, the expected aliases, nothing for a flavor no longer qualified) move to one fast test, test_golden_holds_every_llvm_flavor, which lowers nothing.
  • rocke.core.ir_golden gains check_golden_flavors(golden_path), the flavor-set part of an all-flavor check_golden call, so a caller that checks flavors one at a time keeps the same coverage. check_golden returns the same results as before; the library golden tests that call it are unchanged.
  • The file's golden tests are now pytest functions (unittest has no parametrization); the other tests in the file stay unittest classes. Running the file directly now runs pytest.
  • The installed-tree golden gate (rocke_installed_golden_test.py, CTest rocke_golden_static) is the same check but runs as its own CTest process with no 60 s per-test limit, so it is left as one all-flavor call.
  • BUILDING.md drops its note that the golden test reports conv drift without the C++ engine: the Python lowerer matches the goldens at every flavor.

No emitter, IR or golden changes.

Test Plan

  • Coverage equality: with lowering replaced by a recorder, count the (case, flavor) pairs the old single test and the new per-flavor tests lower, and compare them with the golden's entries.
  • New unit tests in tests/core/test_ir_golden.py: check_golden_flavors accepts a correct golden and reports a stale profile document; checking each flavor separately plus check_golden_flavors gives exactly the errors of one all-flavor check_golden call.
  • The new tests and the old test pass on the unchanged golden; rocke_installed_golden_test.py passes.
  • Byte identity at llvm20, llvm22, llvm23, llvm24; ctest; full platform suite against develop; pre-commit.

Test Result

  • Coverage: old single test lowers 1440 (case, flavor) pairs (360 cases x 4 flavors); the new tests lower the same 1440 pairs (identical multiset), equal to the golden's 1440 entries.
  • tests/core/test_ir_golden.py and tests/test_rocke_ci_static.py pass (both engines, strict).
  • rocke_installed_golden_test.py passes at all four flavors.
  • Byte identity GREEN at llvm20, llvm22, llvm23 and llvm24; ctest 35/35; pre-commit clean.
  • Full platform suite (tests/core, python/rocke/tests, tests/test_rocke.py, tests/test_rocke_ci_static.py): no failures. The remaining platform test files and the library tests that reach the touched code show no new failures against develop.

🤖 Generated with Claude Code

@therock-pr-bot

therock-pr-bot Bot commented Oct 31, 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

Copy link
Copy Markdown

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

@cgmillette

Copy link
Copy Markdown
Contributor Author

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

PR #14820 — check the representative-IR golden one LLVM flavor per test (cgmillette, AICK-2895) — 🟡 First-round: Coverage equal; CI pending

Prior item Status
Reviewers 🟢 None yet
• code-owner teams requested: rocke-platform, rocke-lib
Develop issue 14733 (golden test timeout) 🟡 OPEN: this PR is the fix candidate
• same 1440 case-flavor pairs; check_golden same on valid alias sets (ran)
• slowest split test 26% of the old one (ran)
• needs gfx94X margin over several runs: pending
Bots / CI 🟡 running
• 35 ✓ · 3 running (math-libs builds) · tests not started
• fails: Multi-Arch CI Summary — first run, cancelled by the label re-run (read)
• required: pre-commit, Component, Math ✓; Multi-Arch pending
Our checks 🟢 pass
• merge vs d1de2108a7 clean; ctest 34 ✓ / 1 skip (ran)
• goldens, byte identity, dg, 31 tests, installed layout ✓ (ran)
• open 12167 conflicts in test_rocke_ci_static.py (ran)
# Sev file:line Finding
N1 🟡 rocke/BUILDING.md:124 Edited note still promises conv drift without the C++ engine
• impact: no drift at any flavor without the engine (ran)
• fix: drop the residual paragraph

Utility: 🟢 Positive — splits the near-limit golden test four ways; same checks, no IR change


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, golden gate (4 flavors), byte identity (llvm22), differential golden, the PR's tests, a coverage recorder (old vs new tests), an old-vs-new check_golden differential, an installed-layout run, the library golden tests, a same-time timing A/B, merges with open PRs on the same files. Flags whether a closer look is needed; not an approval or rejection.

The representative-IR golden test lowered and hashed every case at every
LLVM flavor inside one test, which came close to the per-test timeout of
the installed pytest lane. It is now one test per flavor, parametrized over
GOLDEN_FLAVORS (the qualified-lane list itself), plus one test that checks
the golden's flavor documents and aliases without lowering anything.
Together they check every case at every flavor against the same golden
shards, and a drift message names its flavor.

ir_golden gains check_golden_flavors, the flavor-set part of an all-flavor
check_golden call, so a caller that checks flavors one at a time keeps the
same coverage. check_golden's results are unchanged.

BUILDING.md drops its note that the golden test reports conv drift
without the C++ engine: the Python lowerer matches the goldens at every
flavor, so the note no longer holds.

Co-Authored-By: Claude <noreply@anthropic.com>
@cgmillette
cgmillette force-pushed the users/cgmillette/rocke/golden-test-per-flavor branch from 6bf24a8 to a2fea5b Compare November 2, 2026 02:45
@cgmillette

Copy link
Copy Markdown
Contributor Author

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

PR #14820 — check the representative-IR golden one LLVM flavor per test (cgmillette, AICK-2895) — 🟡 Follow-up: N1 fixed; CI running

Prior item Status
Develop issue 14733 (golden test timeout) 🟢 Done in CI 6bf24a89
• gfx94X shards 1–2: 4 per-flavor tests ✓, each ≤ 24% of the per-test limit (read)
• test code unchanged at head; its gfx94X run pending
Our 10-31 N1: BUILDING.md promises conv drift without the engine 🟢 FIXED a2fea5b4
• paragraph removed; datalayout note and closing line kept (ran)
• Python-lowerer goldens ✓ ×4: gate (ran), CI gfx94X (read)
shumway (11-02) approval 🟢 APPROVED 6bf24a89
• older head; still counts on develop
• since then: only the BUILDING.md paragraph removal (ran)
shumway (11-02) BUILDING.md:122: remove the stale paragraph 🟢 FIXED a2fea5b4
• paragraph removed (ran)
• thread resolved by cgmillette
Bots / CI 🟡 running
• running: Math Libs builds ×4, HipDNN Superbuild ×2; no test jobs yet
• 19 ✓ · 0 ✗; duplicate therock-pr-bot run cancelled; its later run ✓
• merge: blocked; required Multi-Arch CI Summary pending
Our checks 🟢 pass
• merge vs 650a69da clean; ctest 34 ✓ / 1 skip; 31 tests ✓ (ran)
• byte identity, golden gate, differential golden ✓ ×4; rules clean (ran)
• open 12167 still conflicts in test_rocke_ci_static.py (ran)

Net-new: none.

Utility: 🟢 Positive — splits the near-limit golden test four ways; CI shows ample margin


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: range-diff vs 6bf24a89, merge with develop, build + ctest, golden gate and byte identity (4 flavors), differential golden, the PR's tests, gfx94X CI logs at 6bf24a89, merge with open 12167. Flags whether a closer look is needed; not an approval or rejection.

@aosewski aosewski 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.

Small comment to improve doc - not blocking.

@aosewski
aosewski merged commit 412a515 into develop Nov 2, 2026
164 of 167 checks passed
@aosewski
aosewski deleted the users/cgmillette/rocke/golden-test-per-flavor branch November 2, 2026 10:33
cgmillette added a commit that referenced this pull request Nov 2, 2026
#14862)

## Motivation

Follow-up to #14820, from @aosewski's review there. #14820 removed the
BUILDING.md note that 6 `conv/*` golden cases drift when the C++ engine
is not built, because the claim is false: the golden check (`lower_case`
in `tests/instances/rocke_ir_parity_harness.py`) always lowers with the
Python lowerer at an explicit LLVM flavor, so the hashes do not depend
on the engine, and the per-flavor golden tests pass with `rocke_engine`
not importable. The same note was still in the "known residuals" callout
of `KERNEL_AUTHORING.md`, pointing at BUILDING.md, which no longer
mentions it.

## Technical Details

- `KERNEL_AUTHORING.md`: the callout keeps only the datalayout residual
(wording as suggested in the review).

Docs only.

## Test Plan

- No other `.md` under `rocke/` mentions the conv residual (searched).

## Test Result

- pre-commit passes.

ISSUE ID : AICK-2895

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants