Skip to content

feat(rocprofiler-compute): Run catalog skill evals on Strix Halo - #15077

Merged
vedithal-amd merged 3 commits into
developfrom
users/feizheng10/rocprof-compute-extended-evals
Nov 6, 2026
Merged

vedithal-amd merged 3 commits into
developfrom
users/feizheng10/rocprof-compute-extended-evals

Conversation

@feizheng10

@feizheng10 feizheng10 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Keep every case that passes on Strix Halo in evals/evals.json, the only file amd/skills runs, on its Strix Halo runners. Move what those runners cannot run to evals/extended_evals.json: the MI300X and gfx950 memory cases, the occupancy.hip name filter, and the torch-trace profile cases, which need PyTorch.
  • Add Strix Halo versions of the memory hit-rate and bandwidth cases and of the kernel-name filter. Only pc-sampling still asks for MI300X in machine.yml.
  • run_evals.py runs both datasets. The minimum case count applies to evals.json only.
  • Remove the note and comment fields, which no runner reads.
  • Split the skills README into this repo's workflow and the rules amd/skills sets.

AIPROFCOMP-965

Test plan

  • ./skills/run_evals.py --list validates both datasets for every skill
  • skillscope v0.1.3 structural checks pass
  • The samples that evals.json uses compile for gfx1151
  • mem-access-inefficiency behavioral case passes locally on MI350X (gfx950)
  • Behavioral run on Strix Halo through amd/skills

Made with Cursor

🤖 Generated with Claude Code

The AMD skills catalog grades evals.json only. Move the sample profile cases into extended_evals.json so that run stays a trigger check, and grade the long cases here with --extended.

Co-authored-by: Cursor <cursoragent@cursor.com>
@feizheng10
feizheng10 requested review from a team and prbasyal-amd as code owners November 5, 2026 01:57
Copilot AI balanced review requested due to automatic review settings November 5, 2026 01:57
@therock-pr-bot

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

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocprofiler-compute/skills/run_evals.py; no test file found
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 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 5, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

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 are reported for the change, which keeps default evaluations short while preserving opt-in profiling coverage.

No open issues 🎉

What changed in this PR

This PR keeps rocprofiler-compute’s catalog evaluations short by separating sample-profiling cases from the default datasets while preserving opt-in local coverage.

Changes:

  • Move long profile-and-read cases for all six skills into extended_evals.json, retaining short routing and behavioral checks in evals.json.
  • Add --extended to the local runner, validate both datasets, and reject duplicate case IDs across files.
  • Document the dataset split and updated evaluation commands.

Paths below are relative to projects/rocprofiler-compute/skills/.

File Description
run_evals.py Adds extended-case selection, validation, and listing details.
rocprof-compute-torch-trace/​evals/​extended_evals.json Holds two moved PyTorch profiling cases.
rocprof-compute-torch-trace/​evals/​evals.json Retains short torch-trace checks.
rocprof-compute-speed-of-light/​evals/​extended_evals.json Holds two moved efficiency-report cases.
rocprof-compute-speed-of-light/​evals/​evals.json Adds a short peak-performance routing check.
rocprof-compute-roofline/​evals/​extended_evals.json Holds three moved roofline profiling cases.
rocprof-compute-roofline/​evals/​evals.json Adds two short roofline routing checks.
rocprof-compute-pc-sampling/​evals/​extended_evals.json Holds three moved PC-sampling cases.
rocprof-compute-pc-sampling/​evals/​evals.json Adds a short stall-reason routing check.
rocprof-compute-memory/​evals/​extended_evals.json Holds five moved memory profiling cases.
rocprof-compute-memory/​evals/​evals.json Replaces profiling cases with short workflow and routing checks.
rocprof-compute-kernel-bottleneck/​evals/​extended_evals.json Holds four moved kernel profiling cases.
rocprof-compute-kernel-bottleneck/​evals/​evals.json Retains replay-safety coverage and adds short routing checks.
README.md Explains minimum versus extended datasets and how to run them.

Review effort: Balanced

@vedithal-amd vedithal-amd 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.

will review

- evals.json keeps every case that passes on Strix Halo, where amd/skills
  runs it; extended_evals.json holds the MI300X, gfx950, and PyTorch cases
- Add gfx115x memory and kernel-name-filter cases; only pc-sampling still
  asks for MI300X in machine.yml
- run_evals.py runs both datasets; the README separates this repo's workflow
  from the rules amd/skills sets

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@vedithal-amd vedithal-amd changed the title feat(rocprofiler-compute): Keep catalog skill evals short feat(rocprofiler-compute): Run catalog skill evals on Strix Halo Nov 5, 2026

@vedithal-amd vedithal-amd 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.

LGTM. I pushed a commit on top so these evals target Strix Halo:

  • evals.json is the set amd/skills runs on Strix Halo, so it keeps every case that passes there. extended_evals.json only holds what Strix Halo can't run (MI300X and gfx950 memory cases, the occupancy.hip filter, torch-trace profiling).
  • Added gfx115x memory and kernel-filter cases. Only pc-sampling keeps the MI300X label.
  • run_evals.py always runs both files; pick cases with --skill or --only.
  • Dropped note and comment, and split the skills README into this repo vs. amd/skills.

We'll validate the Strix Halo cases through the amd/skills PR.

@vedithal-amd
vedithal-amd merged commit 30c594a into develop Nov 6, 2026
79 of 80 checks passed
@vedithal-amd
vedithal-amd deleted the users/feizheng10/rocprof-compute-extended-evals branch November 6, 2026 04:24
@systems-assistant

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