Skip to content

[BugFix] Fixed a precision issue with one-word answers.#3385

Merged
hsliuustc0106 merged 2 commits into
vllm-project:mainfrom
amy-why-3459:fix_one_word
May 6, 2026
Merged

[BugFix] Fixed a precision issue with one-word answers.#3385
hsliuustc0106 merged 2 commits into
vllm-project:mainfrom
amy-why-3459:fix_one_word

Conversation

@amy-why-3459
Copy link
Copy Markdown
Contributor

@amy-why-3459 amy-why-3459 commented May 6, 2026

PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS (AT THE BOTTOM) HAVE BEEN CONSIDERED.

Purpose

Fix #3341

When the thinker finishes, the talker needs to use an EOS token to mark that the thinker output is complete. The incorrect use of "finished" as the marker caused the request to fail to concatenate EOS, resulting in the talker outputting extra tokens.

image

Test Plan

pytest -sv tests/e2e/online_serving/test_qwen3_omni_expansion.py::test_one_word_prompt_001 -m "full_model" --run-level "full_model" --count=30

Test Result

=============================== warnings summary ===============================
<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: builtin type SwigPyPacked has no __module__ attribute

<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: builtin type SwigPyObject has no __module__ attribute

../../../usr/local/lib/python3.12/dist-packages/torch/jit/_script.py:365: 14 warnings
  /usr/local/lib/python3.12/dist-packages/torch/jit/_script.py:365: DeprecationWarning: `torch.jit.script_method` is deprecated. Please switch to `torch.compile` or `torch.export`.
    warnings.warn(

tests/e2e/online_serving/test_qwen3_omni_expansion.py::test_one_word_prompt_001[async_chunk-1-30]
  /usr/local/lib/python3.12/dist-packages/pydub/utils.py:14: DeprecationWarning: 'audioop' is deprecated and slated for removal in Python 3.13
    import audioop

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
--- Running Summary
================= 30 passed, 17 warnings in 1264.33s (0:21:04) =================

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan. Please provide the test scripts & test commands. Please state the reasons if your codes don't require additional test scripts. For test file guidelines, please check the test style doc
  • The test results. Please paste the results comparison before and after, or the e2e results.
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model. Please run mkdocs serve to sync the documentation editions to ./docs.
  • (Optional) Release notes update. If your change is user-facing, please update the release notes draft.

BEFORE SUBMITTING, PLEASE READ https://github.com/vllm-project/vllm-omni/blob/main/CONTRIBUTING.md (anything written below this line will be removed by GitHub Actions)

@gcanlin gcanlin added the omni-test label to trigger buildkite omni model test in nightly CI label May 6, 2026
@hsliuustc0106 hsliuustc0106 removed the omni-test label to trigger buildkite omni model test in nightly CI label May 6, 2026
Signed-off-by: amy-why-3459 <wuhaiyan17@huawei.com>
similarity = cosine_similarity_text(audio_content.lower(), text_content.lower())
print(f"similarity is: {similarity}")
assert similarity > 0.9, "The audio content is not same as the text"
assert similarity > 0.8, "The audio content is not same as the text"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why relax to 0.8?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why relax to 0.8?

After communicating with @yenuo26 , in order to eliminate the influence of whisper, we relaxed the similarity to 0.8.

@hsliuustc0106 hsliuustc0106 added the omni-test label to trigger buildkite omni model test in nightly CI label May 6, 2026
Comment thread vllm_omni/model_executor/models/qwen3_omni/qwen3_omni.py Outdated
Comment thread vllm_omni/model_executor/models/qwen3_omni/qwen3_omni.py Outdated
@oglok
Copy link
Copy Markdown
Contributor

oglok commented May 6, 2026

I closed the temperature fix, I think this is a better approach. However the PR seems a bit unstructured.

  • All the commented code and tests for Bagel are unrelated to this issue
  • If this fix works, there is no need to increase the amount of retries from 3 to 10.
  • It'd be good to explain in the PR description that there is a collision between the stage_input_processors pipeline meta keys and the model layer.

@amy-why-3459
Copy link
Copy Markdown
Contributor Author

I closed the temperature fix, I think this is a better approach. However the PR seems a bit unstructured.

  • All the commented code and tests for Bagel are unrelated to this issue
  • If this fix works, there is no need to increase the amount of retries from 3 to 10.
  • It'd be good to explain in the PR description that there is a collision between the stage_input_processors pipeline meta keys and the model layer.

Thank you very much for your suggestion. I will add a description of the changes in the PR.

hsliuustc0106

This comment was marked as off-topic.

Copy link
Copy Markdown
Collaborator

@hsliuustc0106 hsliuustc0106 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING:

Scope - This PR mixes the core bugfix with unrelated changes: commented Bagel tests, test threshold relaxations, and retry increases. The key collision fix looks reasonable, but please address the structural issues flagged by oglok:

  1. Remove commented Bagel code and test changes not related to this issue
  2. Explain why the meta key change avoids collision with stage_input_processors pipeline in the PR description
  3. If the EOS fix is correct, the retry increase from 3 to 10 should not be needed
  4. Justify the similarity threshold changes from 0.9 to 0.8

Co-authored-by: Canlin Guo <961750412@qq.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
@hsliuustc0106 hsliuustc0106 merged commit b25ea13 into vllm-project:main May 6, 2026
4 of 6 checks passed
@oglok
Copy link
Copy Markdown
Contributor

oglok commented May 7, 2026

It's already merged but the suggestions were not addressed.

@amy-why-3459
Copy link
Copy Markdown
Contributor Author

It's already merged but the suggestions were not addressed.

  1. Remove commented Bagel code and test changes not related to this issue

    The test cases for bagel were removed because they were not ready and had been commented out.

  2. Explain why the meta key change avoids collision with stage_input_processors pipeline in the PR description

    Description has been added to the PR

  3. If the EOS fix is correct, the retry increase from 3 to 10 should not be needed

    The number of retries was changed from 3 to 10 to eliminate the effect of whisper.

  4. Justify the similarity threshold changes from 0.9 to 0.8
    The threshold was changed from 0.9 to 0.8, also to eliminate the effect of whisper.

bb914ef919ad0be5de4c3ad285ed3b2e

The third and fourth points concern the modification of the test cases, which has been agreed upon with the test case owner @yenuo26 .

@oglok
Copy link
Copy Markdown
Contributor

oglok commented May 7, 2026

It's already merged but the suggestions were not addressed.

  1. Remove commented Bagel code and test changes not related to this issue
    The test cases for bagel were removed because they were not ready and had been commented out.

These test cases were not related to the purpose of the fix, now, another developer had to write a PR to clean it up. See: #3407

  1. Explain why the meta key change avoids collision with stage_input_processors pipeline in the PR description
    Description has been added to the PR
  1. If the EOS fix is correct, the retry increase from 3 to 10 should not be needed
    The number of retries was changed from 3 to 10 to eliminate the effect of whisper.

Again, if the fix works, we should not need 10 retries. 3 should be enough.

  1. Justify the similarity threshold changes from 0.9 to 0.8
    The threshold was changed from 0.9 to 0.8, also to eliminate the effect of whisper.
bb914ef919ad0be5de4c3ad285ed3b2e The third and fourth points concern the modification of the test cases, which has been agreed upon with the test case owner @yenuo26 .

@amy-why-3459
Copy link
Copy Markdown
Contributor Author

amy-why-3459 commented May 7, 2026

It's already merged but the suggestions were not addressed.

  1. Remove commented Bagel code and test changes not related to this issue
    The test cases for bagel were removed because they were not ready and had been commented out.

These test cases were not related to the purpose of the fix, now, another developer had to write a PR to clean it up. See: #3407

  1. Explain why the meta key change avoids collision with stage_input_processors pipeline in the PR description
    Description has been added to the PR
  1. If the EOS fix is correct, the retry increase from 3 to 10 should not be needed
    The number of retries was changed from 3 to 10 to eliminate the effect of whisper.

Again, if the fix works, we should not need 10 retries. 3 should be enough.

  1. Justify the similarity threshold changes from 0.9 to 0.8
    The threshold was changed from 0.9 to 0.8, also to eliminate the effect of whisper.
bb914ef919ad0be5de4c3ad285ed3b2e The third and fourth points concern the modification of the test cases, which has been agreed upon with the test case owner @yenuo26 .

Thank you very much for your suggestion. However, this test case was originally supposed to be deleted. I simply commented it out and am waiting for @fake0fan to completely remove it. You can check out the comments on this PR: #2396.

clodaghwalsh17 pushed a commit to clodaghwalsh17/nm-vllm-omni-ent that referenced this pull request May 12, 2026
…#3385)

Signed-off-by: amy-why-3459 <wuhaiyan17@huawei.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Co-authored-by: Canlin Guo <961750412@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

omni-test label to trigger buildkite omni model test in nightly CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [CI failure]: CI audio-text consistency assertion fails intermittently

4 participants