Skip to content

[fix] Remove redundant parameter passing#90

Merged
Gaohan123 merged 3 commits intomainfrom
qby_dev/fix_param
Nov 27, 2025
Merged

[fix] Remove redundant parameter passing#90
Gaohan123 merged 3 commits intomainfrom
qby_dev/fix_param

Conversation

@qibaoyuan
Copy link
Contributor

@qibaoyuan qibaoyuan commented Nov 26, 2025

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

Purpose

Remove unnecessary parameter passing to simplify the code.

Test Plan

cli as given

Test Result

  1. online
vllm-omni serve  /mnt/user/qibaoyuan/Qwen2.5-Omni-3B --omni --port 8791 \
--stage-configs-path ~/qwen2_5_omni.yaml

result:
截屏2025-11-27 18 43 19
截屏2025-11-27 18 45 05

  1. offline
    3.1 text
截屏2025-11-27 20 03 05

3.2 use_audio_in_video
截屏2025-11-27 20 13 52


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, such as providing test command.
  • [x ] The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (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)

Signed-off-by: 齐保元 <qibaoyuan@xiaomi.com>
@chatgpt-codex-connector
Copy link

💡 Codex Review

log_file=model._log_file
in_q=model._in_q
out_q=model._out_q

P0 Badge Use stage queues instead of model string in sync worker

_stage_worker now pulls _log_file, _in_q, and _out_q from model (lines 243‑245), but init_stage_worker calls it with the model name string from _start_stage_processes. On the synchronous path this immediately raises AttributeError before the worker can log readiness or process batches, so every stage process crashes and the orchestrator never runs.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Signed-off-by: 齐保元 <qibaoyuan@xiaomi.com>
Signed-off-by: 齐保元 <qibaoyuan@xiaomi.com>
@qibaoyuan
Copy link
Contributor Author

💡 Codex Review

log_file=model._log_file
in_q=model._in_q
out_q=model._out_q

P0 Badge Use stage queues instead of model string in sync worker
_stage_worker now pulls _log_file, _in_q, and _out_q from model (lines 243‑245), but init_stage_worker calls it with the model name string from _start_stage_processes. On the synchronous path this immediately raises AttributeError before the worker can log readiness or process batches, so every stage process crashes and the orchestrator never runs.

ℹ️ About Codex in GitHub

thx, fixed

Copy link
Collaborator

@Gaohan123 Gaohan123 left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@Gaohan123 Gaohan123 merged commit 38f5806 into main Nov 27, 2025
2 of 3 checks passed
@hsliuustc0106 hsliuustc0106 deleted the qby_dev/fix_param branch November 28, 2025 01:08
princepride pushed a commit to princepride/vllm-omni that referenced this pull request Jan 10, 2026
Signed-off-by: 齐保元 <qibaoyuan@xiaomi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants