Repository navigation
fix(tensilelite): fail the LibraryClient step when the library selects no solution - #15254
Conversation
github-actions
Bot
added
project: hipblaslt
project: hipsparselt
ci:hipsparselt-fast
labels
262e21a to
3c6d28e
Compare
✅ 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. |
Alex-Vasile
removed
NoCI
…s no solution With --best-solution the client ran library solution 0 when selection returned nothing, and Tensile ignored the LibraryClient return code, so a library that selects nothing (for example a broken Prediction setup) passed. A new client option --require-selected-solution (passed only on the LibraryClient run) reports and counts such problems, and Tensile now fails on a non-zero LibraryClient exit under ExitOnFails. One line is hand-written (user-approved): SolutionIterator::Default() passes the new argument in base's two-line make_shared<TopSolutionIterator> call; branch 10 (predicate dump opt-in) reflows that call into its final form.
3c6d28e to
6f5d536
Compare
tony-davis
left a comment
tony-davis
left a comment
There was a problem hiding this comment.
Hi Alex! I think this is an important fix. A LibraryClient step that quietly runs solution 0 when selection comes back empty is exactly the kind of thing that makes a green test mean less than people think. I traced the new path and it looks right to me. With --require-selected-solution, TopSolutionIterator skips the problem, counts it once, and that count reaches main's exit code through MetaRunListener::error(), the same way a validation failure does. Without the option, nothing changes. I also checked that the hw-common job takes the client and the tests from the same installed artifact, so I don't see a lane where an older client would trip over the new option.
The tests are good. NothingSelectedIsAnErrorWithTheOption uses a real MasterSolutionLibrary and would fail if the option check, the count, or the log line went away. test_failed_library_client_run_fails_tensile would fail if the return-code check were dropped.
Things I'd like to see before merge:
- Most of the affected configs are gfx950. On develop, 12 common configs have a
LibraryClientstep. 11 are gfx950-only (fused_a2a, the 8subtile_mxfp8_*configs, and the 2mxfp8_*_reproconfigs), and onlyxfp32.yamlis gfx942. The Test Plan runs only-k xfp32. TheRock'stensilelite-commonjob runs gfx950 per PR, so I think it's enough to point at that job's result once it's green. I'd add those configs to the Test Plan so a reader knows they're the ones at stake. prediction_bf16isn't in the tree. The Test Result cites it, but I couldn't find it on develop. Could you say which in-tree config shows the same thing, or note that it was a scratch config?- Which lane runs the new gtests? The overlay wants defect-fix regression tests to run in a shared lane.
tests/test_categories.yamldefines astandardtier labelledpr, but I couldn't confirm from here that a required check actually runstensilelite-testson a PR. (Math CI'scoverage-cppruns ctest, but it doesn't gate.) If you know the lane, a link to its run withSolutionIteratorSelectionTestin it would settle it.
Smaller things:
mxfp8_residual_fail_classes_repro.yamlsays it fails "only with some probability." The LibraryClient run now gates too, so that config gets two chances to flake per run. Is it stable in CI today?- The option does nothing without
--best-solution, because onlyTopSolutionIteratorreads it. The help text says so, but I bet a one-line warning would save someone a confusing afternoon. - Same note as the earlier layers on the "must be shown to fail" sentence in the Test Plan. I'd say what was shown locally and what is pending.
Suggested replacement for the device paragraph:
Client host code (solution iteration) and the Python test harness; no kernel generation changes. Twelve common configs end with a LibraryClient step:
xfp32.yamlon gfx942, and 11 gfx950 configs (comm/gfx950/fused_a2a, eightsubtile_mxfp8_*,mxfp8_verify_fail_reproandmxfp8_residual_fail_classes_repro). Each one now fails if the library selects nothing or the client exits non-zero. Before merge, TheRock'stensilelite-commonjob must pass on gfx942 and gfx950. gfx950 so far has only a local run of an F32 copy ofxfp32.yaml.
Thanks!
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (70.07%) 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 @@
## users/alvasile/tl-speedup-03-codegen-worker-metadata #15254 +/- ##
=====================================================================================
Coverage 78.10% 78.10%
=====================================================================================
Files 3007 3007
Lines 522273 522270 -3
Branches 81515 81515
=====================================================================================
- Hits 407877 407875 -2
+ Misses 85449 85448 -1
Partials 28947 28947
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
JIRA ID : AIHPBLAS-5255
Motivation
A Tensile test config can end with a LibraryClient step. It writes library logic, builds a solution library from it, and runs tensilelite-client, the C++ program that runs and validates generated GPU kernels, in
--best-solutionmode, so the library itself picks the kernel for each problem. That step is how the tests check solution selection. When selection returned nothing, the client ran library solution 0 instead and exited 0. Tensile also ignored the LibraryClient run's return code, so a broken selection (for example, a Prediction library with no hardware model) passed its test.Technical Details
Production code:
main.cpp,SolutionIterator.hpp,SolutionIterator.cpp): a new option--require-selected-solution(defaultfalse). With it,TopSolutionIterator, the iterator--best-solutionuses, does not fall back to solution 0 when selection is empty. It logs[TopSolutionIterator] No solution selected for problem <op> sizes <...>at Error level, skips the problem and counts it in a newerror()override. The client then exits non-zero, through the same channel as a validation failure. Without the option, behavior is unchanged.ClientWriter.writeRunScriptpasses the option only on the LibraryClient run. Benchmark runs do not get it.Tensile.executeStepsInConfignow keepsClientWriter.main's return code for the LibraryClient run. A non-zero code ends Tensile withLibraryClient exited with code NwhenExitOnFailsis set (default1), as the benchmark step already does. TheExitOnFailscomment inGlobalParameters.pysays so.Test code:
tests/SolutionIteratorSelection_test.cpp(registered intests/CMakeLists.txt) has 3 host-only gtests throughSolutionIterator::Default(). They cover a selection running with or without the option, the solution-0 fallback without it, and skip, log and count with it.Tests/unit/test_library_client_require_selection.pyhas 5 tests: the option is written on LibraryClient lines and not on benchmark lines, a return code of 1 ends Tensile, and0orExitOnFails: 0do not.Behavior change: every test config with a
LibraryClientsection now fails when selection is empty, or when the LibraryClient run exits non-zero for any other reason. A client binary older than this change aborts on the unknown option.Test Plan
From
projects/hipblaslt/tensilelite. C++ tests use one configured build:cmake --preset tensilelite -S .. -B <build> -DTENSILELITE_BUILD_TESTING=ONandcmake --build <build> --parallel.tox -e unit -- tensilelite/Tests/unit/test_library_client_require_selection.py tensilelite/Tests/unit/characterizationchecks the run script and the return-code rule, plus the existingClientWritercharacterization tests.ctest --test-dir <build>/tensilelite/tests --output-on-failure -R 'SolutionIteratorSelection'runs the 3 gtests.tox -e py3 -- tensilelite/Tests -m common -k 'xfp32'on a gfx942 GPU runs the GridBased LibraryClient config, which only runs on gfx942.pre-commit run --files client/include/SolutionIterator.hpp client/main.cpp client/src/SolutionIterator.cpp tensilelite/ClientWriter.py tensilelite/Common/GlobalParameters.py tensilelite/Tensile.py tensilelite/Tests/unit/test_library_client_require_selection.py tests/CMakeLists.txt tests/SolutionIteratorSelection_test.cppruns the repository hooks.Regression tests:
SolutionIteratorSelectionTest.NothingSelectedIsAnErrorWithTheOptionandtest_failed_library_client_run_fails_tensile. Both must be shown to fail without the fix and pass with it, in a shared CI lane (TheRock GitHub Actions or Math CI). The LibraryClient step itself must also pass on gfx942 and gfx950.Test Result
These results were taken on the original unsplit branch, on a gfx950 host with a development build:
prediction_bf16passed with the library's own pick,MT64x64. With a client wrapper that made its problem unmatched, Tensile exited 255 and the pytest run phase failed.ClientWriterrun-path characterization tests passed; the new tests failed against the unfixed Python files.xfp32.yamlwas not run with the option, because it is gfx942-only. A gfx950 F32 copy of it passed.Not yet run on this branch. Automated GitHub checks are pending.
Related
Device / Architecture Coverage
Client host code (solution iteration) and the Python test harness; no kernel generation changes. The LibraryClient step runs on the GPU, and
xfp32.yamlruns only on gfx942. Before merge, the common tests' LibraryClient configs need a passing run on gfx942 and gfx950 in CI.Submission Checklist
Risk level
Moderate (3/5): any non-zero LibraryClient exit now fails Tensile, including validation failures that used to pass with a warning and clients built from an older tree. The gfx942-only
xfp32LibraryClient step has not been run with the option, and the full unit suite has not completed.