Skip to content

[AIROCMLIR-1061] Increase rocm-smi timeout#2428

Open
justinrosner wants to merge 1 commit into
developfrom
1061-rocm-smi-timeout
Open

[AIROCMLIR-1061] Increase rocm-smi timeout#2428
justinrosner wants to merge 1 commit into
developfrom
1061-rocm-smi-timeout

Conversation

@justinrosner

Copy link
Copy Markdown
Contributor

Motivation

This PR addresses some flaky/intermittent behavior observed in the tuningRunner-* LIT tests when calling into rocm-smi for GPU topology discovery.

Technical Details

In https://github.com/ROCm/rocmlirTriton/pull/310 in rocmlirTriton we bumped the timeout from 10s -> 60s. Looking at the history of nightly CI runs since then shows that this issue has not come up. This PR just ports those same changes to rocMLIR.

Test Plan

  • Run the nightly CI

Test Result

  • Nightly CI

Submission Checklist

@justinrosner justinrosner requested a review from causten as a code owner July 13, 2026 15:46
Copilot AI review requested due to automatic review settings July 13, 2026 15:46
@justinrosner justinrosner changed the title [AIROCMLIR=1061] Increase rocm-smi timeout [AIROCMLIR-1061] Increase rocm-smi timeout Jul 13, 2026
@justinrosner justinrosner added the claude-review Trigger automated PR review by claude[bot]; auto-removed after the run. label Jul 13, 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.

Pull request overview

This PR reduces flakiness in tuningRunner-* LIT tests by increasing the rocm-smi GPU topology discovery timeout, aligning rocMLIR with the previously proven change in rocmlirTriton.

Changes:

  • Increase rocm-smi invocation timeout from 10s to 60s during GPU topology discovery.
  • Add an inline comment explaining why a larger timeout is necessary on large multi-GPU systems.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rocmlir-pr-reviewer rocmlir-pr-reviewer 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.

Verdict: APPROVE -- submitted as COMMENT (automated reviews are advisory)  ·  Findings: 0 (0 Critical, 0 Major, 0 Minor)


Scope

Bumps the rocm-smi subprocess timeout in GpuTopology.discover() from 10s to 60s and adds an explanatory comment, addressing flaky tuningRunner-* LIT tests. Single-file, single-line functional change (mlir/utils/performance/tuningRunner.py), porting a fix already validated in rocmlirTriton.

Findings

No blocking issues found.

Notes

  • The added comment cites ~20s enumeration time while the timeout is set to 60s; the extra headroom is reasonable given the flakiness motivation, so no change requested.
  • Change is a pure constant bump on an existing, tested code path; no new op/pass/API surface, so no additional Lit/E2E coverage is warranted.

CI status

No non-self checks report fail or cancel. Python format/lint and Python performance script tests pass; C/C++ premerge is still in progress at review time.

@rocmlir-pr-reviewer rocmlir-pr-reviewer Bot removed the claude-review Trigger automated PR review by claude[bot]; auto-removed after the run. label Jul 13, 2026
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2428      +/-   ##
===========================================
+ Coverage    82.57%   82.86%   +0.30%     
===========================================
  Files          120      120              
  Lines        42852    42828      -24     
  Branches      7110     7106       -4     
===========================================
+ Hits         35381    35489     +108     
+ Misses        4815     4696     -119     
+ Partials      2656     2643      -13     
Flag Coverage Δ
gfx120x 82.74% <ø> (+0.22%) ⬆️
gfx950 82.68% <ø> (+0.34%) ⬆️
mfma 82.63% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

4 participants