Skip to content

test: validate MI300 runtime selection and placement - #1045

Merged
ndizazzo merged 2 commits into
codex/issue-966-rocm-kernel-fallbackfrom
codex/issue-653-mi300-utilization
Jul 22, 2026
Merged

test: validate MI300 runtime selection and placement#1045
ndizazzo merged 2 commits into
codex/issue-966-rocm-kernel-fallbackfrom
codex/issue-653-mi300-utilization

Conversation

@ndizazzo

@ndizazzo ndizazzo commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add a regression test proving MI300X gfx942 KFD evidence selects a compatible ROCm native runtime instead of the CPU fallback
  • add a planner regression test proving a 192 GiB MI300X receives the capacity-weighted majority of a large model split
  • document through executable coverage that the under-utilization in GPU not utilized on test MI300 host #653 is fixed by the ROCm bootstrap work in fix: ROCm GPU enumeration #1039, without adding a device-name-specific scheduling heuristic

Root cause

The issue logs show every model buffer allocated as CPU_Mapped, while the MI300X (1002:74b5) remained idle. Runtime artifact selection occurred before authoritative HIP enumeration. Without early ROCm architecture evidence, release builds could select the CPU native runtime, after which no placement decision could use the GPU. #1039 discovers gfx_target_version from KFD topology first (90402gfx942) so the ROCm runtime is loaded. Once that capability is present, the existing resource planner already weights placement by available accelerator memory.

This PR is intentionally stacked on #1039 and adds the MI300-specific end-to-end decision coverage requested by #653.

Validation

  • cargo test -p mesh-llm-hardware-profile --lib
  • cargo test -p mesh-llm-host-runtime runtime::split_planning::tests --lib
  • cargo check -p mesh-llm-hardware-profile -p mesh-llm-host-runtime -p mesh-llm
  • cargo clippy -p mesh-llm-hardware-profile -p mesh-llm-host-runtime -p mesh-llm --all-targets -- -D warnings
  • just test-all

Depends on #1039. Validates the MI300 path reported in #653.

Summary by CodeRabbit

  • Tests
    • Added coverage confirming ROCm is selected when MI300X hardware evidence is detected and no alternative accelerator runtimes are available.
    • Added coverage verifying resource-aware model splitting assigns proportionally more model data to higher-capacity MI300X hardware.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e9f6cd49-4c39-48f6-8336-d6863f213e29

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds unit tests covering ROCm selection from KFD evidence and capacity-weighted split planning for an MI300X compared with a smaller accelerator.

Changes

ROCm runtime selection

Layer / File(s) Summary
KFD-based ROCm selection test
crates/mesh-llm-hardware-profile/src/lib.rs
Tests that recommended selection chooses a ROCm runtime artifact over CPU when ROCm is inferred from KFD architecture evidence.

Resource-aware split planning

Layer / File(s) Summary
MI300X capacity allocation test
crates/mesh-llm-host-runtime/src/runtime/split_planning.rs
Tests two-stage planning, MI300X-first assignment, and allocation of at least three times the parameter bytes assigned to the smaller accelerator.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: michaelneale, michaelneale, i386

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main change: tests for MI300 runtime selection and split placement.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-653-mi300-utilization

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions
github-actions Bot requested a review from i386 July 21, 2026 19:46
@ndizazzo

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/mesh-llm-hardware-profile/src/lib.rs`:
- Around line 719-721: Replace the EnvVarGuard-based environment mutation in the
ROCm detection tests with a pure helper that accepts the ROCm architecture
values directly. Update detect_rocm_profile_with_arches and the related tests to
reuse this helper, including the cases currently using EnvVarGuard, while
preserving each test’s expected profile behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a65ff85a-72de-4f0a-bdee-6b5300501414

📥 Commits

Reviewing files that changed from the base of the PR and between 4e101fb and bff621f.

📒 Files selected for processing (2)
  • crates/mesh-llm-hardware-profile/src/lib.rs
  • crates/mesh-llm-host-runtime/src/runtime/split_planning.rs

Comment thread crates/mesh-llm-hardware-profile/src/lib.rs Outdated
@ndizazzo
ndizazzo force-pushed the codex/issue-653-mi300-utilization branch from e5278c8 to 3f4aa50 Compare July 21, 2026 20:41
@ndizazzo
ndizazzo changed the base branch from codex/issue-651-rocm-gpu-enumeration to codex/issue-966-rocm-kernel-fallback July 21, 2026 20:44
@ndizazzo
ndizazzo force-pushed the codex/issue-653-mi300-utilization branch from 3f4aa50 to 5924fe1 Compare July 22, 2026 00:07
@ndizazzo
ndizazzo merged commit ce0222c into main Jul 22, 2026
29 of 39 checks passed
@ndizazzo
ndizazzo deleted the codex/issue-653-mi300-utilization branch July 22, 2026 01:21
michaelneale added a commit that referenced this pull request Jul 22, 2026
…ngram

* origin/main:
  Add GLM DSA correctness trace reporting (#1033)
  Add GLM DSA package contract validation (#1032)
  Add direct quant layer package tooling (#1031)
  Lock split topology placement (#1050)
  fix nightly stability Qwen thinking
  ui(data-mode): migrate legacy harness storage to v2 (#1040)
  fix: Windows Vulkan runtime dependencies (#1046)
  test: validate MI300 runtime selection and placement (#1045)
  fix: support ROCm RDNA APU kernels safely (#1044)
  fix: ROCm GPU enumeration (#1039)
  Document public packaging installation channels
  Fix Windows autoupdate import (#1030)
  Update llama.cpp upstream pin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant