Fix NameError on every async query in the llama2-70b API SUT - #2663
Open
David-Wu1119 wants to merge 1 commit into
Open
Fix NameError on every async query in the llama2-70b API SUT#2663David-Wu1119 wants to merge 1 commit into
David-Wu1119 wants to merge 1 commit into
Conversation
async_process_query ends with sys.exit() to finish the worker thread,
but SUT_API.py never imports sys. The thread therefore dies with
NameError: name 'sys' is not defined
and, since the function is a thread target, Python's threading
excepthook prints a traceback for every query. The LoadGen response is
already submitted at that point, so the run still completes, but the
intended exit never happens and the log fills with tracebacks.
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.
Summary
language/llama2-70b/SUT_API.pycallssys.exit()but never importssys:async_process_queryis used as a thread target:so instead of the intended
SystemExit, each worker dies withand Python's
threading.excepthookprints a traceback for every query.lg.QuerySamplesComplete(response)has already run by then, so a run still completes and the numbers are unaffected — but the exit never happens as written and the log fills with tracebacks.ruff --select F821flags it, and the file has noimport sysanywhere (grep -c "import sys" -> 0).Fix
Add
import systo the import block, so the call does what it says.Notes
I kept this to the import rather than touching the
sys.exit()itself: I can't tell from the code whether exiting the worker there is deliberate or a leftover, and adding the import makes the file behave as written without guessing. If it is a leftover, removing the line would be the better change and I'm happy to do that instead — a maintainer who knows the intent should make that call.Heads up on overlap: #2391 also touches
SUT_API.py(adding multinode support). I checked and it does not addimport sys, so the two are complementary, but they will want rebasing around each other.