Refactor warmup logic and generator mock in OnnxDiscrepancyCheck#2568
Merged
Conversation
added 2 commits
July 9, 2026 11:46
…s not consume tokens from the real generation run
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a GenAI warmup “token pollution” bug in OnnxDiscrepancyCheck.compare_generation that could misalign generated-token comparisons (systematically shifting GenAI tokens by one) and thereby corrupt first_token_matches, second_token_matches, and longest_common_token_sequence results.
Changes:
- Refactored GenAI warmup to append into a throwaway
warmup_tokenslist instead ofgenai_tokens, preventing warmup output from affecting the real run. - Adjusted the warmup break condition to be based on generated-token count (via
genai_prompt_token_count + 1threshold) rather than a fixed total-token length. - Updated unit tests to mock
og.Generatorwith aside_effectfactory so each generator instance (warmup + real) replays the deterministic token stream from the start, matching real behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
olive/passes/onnx/discrepancy_check.py |
Prevents warmup-generated tokens from being appended into the “real run” accumulator and fixes warmup stopping criteria. |
test/passes/onnx/test_discrepancy_check.py |
Updates generator mocking to model deterministic re-generation per generator instance and keeps prompt-seeding assertions valid. |
xadupre
approved these changes
Jul 15, 2026
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
Fix a warmup pollution bug in
OnnxDiscrepancyCheck.compare_generation(
olive/passes/onnx/discrepancy_check.py) that caused a systematic off-by-onemisalignment in the ONNX Runtime GenAI generated-token list.
Bug
The GenAI warmup loop (introduced in
fe69db6) reused thegenai_tokensaccumulator across both the warmup and the real generation run. The generator
is correctly re-created for the real run, but the accumulator was not reset, so
warmup-generated tokens were already present in
genai_tokenswhen the real runstarted.
Because generation is deterministic, this shifted every generated token by one
position and produced misleading metrics:
first_token_matches→True(warmup token equals the real first token)second_token_matches→False(real first token lands in the second slot)longest_common_token_sequence→ collapsed to1The warmup break condition (
len(genai_tokens) >= 3) also counted prompt tokens,under-warming on prompts of 3 or more tokens.
Fix
warmup_tokensaccumulator during warmup sogenai_tokensisnever touched until the real run — structurally mirroring how the transformers
side already discards its warmup output.
len(warmup_tokens) >= genai_prompt_token_count + 1so it counts only generated tokens.
The real generation block immediately following was already correct and is
unchanged.
Tests
test/passes/onnx/test_discrepancy_check.pyTestCompareGenerationmocks so each
og.Generatorcall (warmup + real run) replays the token streamfrom the start via an
og.Generator.side_effectfactory. This models realdeterministic re-generation, where warmup and the real run produce the same
tokens and the warmup output is discarded. Previously the mocks shared a single
call_countacross both generators, which encoded the buggy behavior.append_tokensassertions now target a shared mock so the "prompt isseeded for both warmup and real run" checks still hold.
test_compare_generation_with_zero_max_new_tokenswas left unchanged — itsgenerator reports
is_doneimmediately and never appended warmup tokens, so itwas unaffected by the bug.
Checklist before requesting a review
lintrunner -aRelease notes
Fixed a bug in the
OnnxDiscrepancyCheckpass where GenAI warmup tokens pollutedthe generated-token comparison, causing incorrect
first_token_matches,second_token_matches, andlongest_common_token_sequenceresults.(Optional) Issue link