Skip to content

test(miopen): explain ConvHipImplicitGemmV4R1WrW FP32 tolerance - #15150

Merged
JonathanLichtnerAMD merged 2 commits into
developfrom
users/jlichtne/ALMIOPEN-3158-v4r1-wrw-tolerance-comment
Nov 6, 2026
Merged

JonathanLichtnerAMD merged 2 commits into
developfrom
users/jlichtne/ALMIOPEN-3158-v4r1-wrw-tolerance-comment

Conversation

@JonathanLichtnerAMD

Copy link
Copy Markdown
Contributor

JIRA ID: ALMIOPEN-3158

Motivation

Follow-up to #14905 (ROCM-23567). The stale /// \todo 40.0f is too much. The solver needs to be checked. in unit_conv_solver_ConvHipImplicitGemmV4R1WrW.cpp implied a possible solver defect.

Technical Details

Replaces the stale todo with a comment stating the tolerance reflects FP32 accumulation order over K = NHoWo = 193600 with zero-mean random data, not a solver defect. Comment-only; no functional change.

Risk Assessment

Risk Level: 🟢 Low

Comment-only change.

Test Plan

Investigation runs on gfx90a (MI210): measure rms relative error vs the GPU reference (accumulates in double) for N=8/16/32/64, with random data and with small-integer data. Nothing to build since the change is comment-only.

Test Result

  • Random data, rms relative error grows with K: N=8: 2.4 eps, N=16: 4.5, N=32: 11.6, N=64: 30.1 eps (deterministic). The N=64 value 3.59e-06 matches the gfx1030 result.
  • Small-integer inputs (exactly representable in FP32): N=8/16/32/64 give exactly zero error at default tolerance, so the kernel computes the correct sum.

@therock-pr-bot

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

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 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 5, 2026

Copy link
Copy Markdown

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

@SreecharanGundaboluAMD SreecharanGundaboluAMD 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

@JonathanLichtnerAMD
JonathanLichtnerAMD merged commit ad7f336 into develop Nov 6, 2026
151 of 160 checks passed
@JonathanLichtnerAMD
JonathanLichtnerAMD deleted the users/jlichtne/ALMIOPEN-3158-v4r1-wrw-tolerance-comment branch November 6, 2026 21:44
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.

2 participants