fix(TEST01): NameError in verify_performance when the loadgen log reports errors - #2666
Open
Anai-Guo wants to merge 1 commit into
Open
fix(TEST01): NameError in verify_performance when the loadgen log reports errors#2666Anai-Guo wants to merge 1 commit into
Anai-Guo wants to merge 1 commit into
Conversation
parse_result_log formatted the warning with `line`, a leftover from when the file scanned the summary as text (compliance/TEST04 still does). The branch only runs when mlperf_log.has_error() is True, so TEST01 verification crashed exactly when the loadgen log reported errors. num_errors() is what has_error() is defined in terms of and what the submission checker uses for this same message.
Contributor
|
MLCommons CLA bot: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
compliance/TEST01/verify_performance.py::parse_result_logformats a warning with a name thatdoes not exist in that scope:
lineis a leftover from when this file scanned the summary as text — the siblingcompliance/TEST04/verify_performance.py:88still does exactly that(
error = line.split(" ", 1)[0].strip()insidefor line in test_file:). TEST01 was moved tothe parsed
MLPerfLogobject and the format argument was not moved with it.The consequence is that TEST01 verification crashes only when the loadgen log actually
reports errors — i.e. precisely the case this warning was written for. On a clean run
has_error()isFalseand the branch is skipped, which is why it has gone unnoticed.Replaying
parse_result_log(lifted out of the file withast) against a stubMLPerfLogfor an Offline run whose log reports 3 errors:
Fix
Use the log object's own error count,
mlperf_log.num_errors()— the accessorhas_error()is itself defined in terms of (log_parser.py:160), and the one used for thisexact message everywhere else in the repo:
tools/submission/log_parser.py:214—"- Number of errors: {:d}".format(mlperf_log.num_errors())tools/submission/submission_checker/checks/performance_check.py:168tools/submission/submission_checker/checks/accuracy_check.py:276This keeps the rendered message identical in shape to TEST04's
(
"WARNING: " + error + " ERROR reported in ...").🤖 Generated with Claude Code