Repository navigation
fix(tensilelite): fail validation on mismatches when print-max <= 0 - #13478
Conversation
github-actions
Bot
added
project: hipblaslt
project: hipsparselt
ci:hipsparselt-fast
labels
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
There was a problem hiding this comment.
🔵 Needs a closer look
The implementation appears correct, but required shared automated checks are still pending.
0 open findings
What changed in this PR
This PR ensures TensileLite validation reports mismatched GPU results as failed even when mismatch printing is disabled.
Changes:
- Decouples failure detection from diagnostic printing in both comparators.
- Adds regression coverage across all 12 supported element types.
Quality assessment: Defect fix, risk 3/5 while required shared automated checks remain pending. The focused C++ unit test directly detects regression; no feature flag is needed.
Suggested description addition:
Risk level
Moderate (3/5): The change is narrowly scoped and has direct regression coverage, but shared automated checks are pending. It changes default validation behavior so mismatches produce
FAILEDwhenprint-maxis non-positive.
| File | Description |
|---|---|
projects/hipblaslt/tensilelite/client/include/ResultComparison.hpp |
Marks mismatches as failures independently of report printing. |
projects/hipblaslt/tensilelite/tests/ResultComparison_test.cpp |
Verifies disabled printing cannot suppress comparator failures. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
FastPointwiseComparison::report() and PointwiseComparison::report() set m_failed only when printReport was true. ReferenceValidator passes printReport = (print-max > 0), and the client defaults print-max to -1, so with print-max <= 0 a mismatch in D was reported as PASSED. Set m_failed whenever there are errors; printReport now only controls the "Found N incorrect values" line. Ini files written by ClientWriter take print-max from ValidationMaxToPrint, which defaults to 4 and is positive in every YAML in this repository, so this only changes results for hand-written ini files and command-line runs. Add PrintMaxZero_MismatchStillFails to ResultComparison_test.
9c6d398 to
da2a81a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (49.67%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #13478 +/- ##
========================================
Coverage 71.99% 71.99%
========================================
Files 2879 2879
Lines 478104 478104
Branches 70852 70852
========================================
Hits 344166 344166
Misses 109972 109972
Partials 23966 23966
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
TheRock Submodule Bump ActivityNewest first
|
Motivation
The TensileLite client runs matrix-multiplication (GEMM) kernels, functions that run on the GPU, and, when validation is on, compares each kernel's output matrix D with a CPU reference. A mismatch should mark that kernel configuration (a "solution") FAILED. When
--print-max, the limit on how many mismatched values the client prints, was 0 or negative, mismatches were counted but never marked as a failure, so the solution was reported as PASSED.--print-maxdefaults to -1, so a run that turned on validation with--num-elements-to-validatebut left--print-maxunset passed regardless of the result.JIRA ID : AIHPBLAS-4534
Technical Details
ReferenceValidatorchecks D withFastPointwiseComparisonand constructs it withprintReport = (print-max > 0). Itsreport(), and the same method inPointwiseComparison, set the failure flag only inside theprintReportbranch. Both now set the flag whenever the mismatch count is nonzero;printReportonly decides whether the "Found N incorrect values" line is printed. The bounds check for writes outside D (InvalidComparison) already reported errors regardless ofprint-maxand is unchanged.Ini files written by
ClientWritertakeprint-maxfromValidationMaxToPrint, which defaults to 4 and is positive in all 247 YAML files in this repository that set it, so those runs report the same results as before. A hand-written ini file or command line withprint-max <= 0now reports FAILED when D has a mismatch.The new
PrintMaxZero_MismatchStillFailscase intests/ResultComparison_test.cppbuilds both comparators with a print limit of 0, feeds them one mismatched element, and checks thaterror()returns true. It runs for each of the 12 element types the validator supports.#12472 rejects
--print-max <= 0in the multi-GPU path it adds to the client, to avoid this bug; that check can be removed once this change lands.Test Plan
Build
tensilelite-testsfor gfx950 on a host without a GPU, first with only the new test added, then with the fix. In both builds, run the*ResultComparison*cases and the full binary.Test Result
PrintMaxZero_MismatchStillFailsfails for all 12 types, with botherror()calls returning false. The other 48ResultComparisoncases pass.ResultComparisoncases pass.tensilelite-testswith the fix: 641 passed, 9 skipped, 8 failed. The same 8 fail without the fix. Seven stop at a HIP device query with error 100 (hipErrorNoDevice);IndexedLibraryParityTest.ResolveByIndexWorksWithoutAnyPriorQueryfinds no solution while the other tests in its suite skip.Submission Checklist