Repository navigation
[Core] Allow Broadcasting Scalars - #9021
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to belong to: docs/design/module/execution_platforms.md, docs/design/module/ar_runtime.md, docs/design/module/diffusion/index.md. Module owners: @gcanlin @tzhouam @david6666666 Routing: @gcanlin via module of the changed files, CODEOWNERS; @tzhouam via module of the changed files, CODEOWNERS; @david6666666 via module of the changed files @alex-jw-brooks, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Dong1017
left a comment
There was a problem hiding this comment.
Reviewed 1a7ab6195447d360bb689c4779c7d8a352f6b9dd.
The shared-scalar handling preserves request boundaries: waveform lists remain per request, while sample-rate metadata reaches both payloads. On the server, the four targeted tests passed, and a production-helper → GPU worker scalar conversion → payload partition check preserved distinct waveforms plus int, float, bool, and zero-dimensional tensor values. The ndim > 0 guard also covers zero-dimensional metadata on the cached-output path. I found no blocking issue in these changes. Connector/wire roundtrip and full serving E2E were not tested.
Signed-off-by: Alex Brooks <albrooks@redhat.com>
1a7ab61 to
e0fc229
Compare
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Contributor intent (required human answers)
1. What outcome do you want to achieve?
I am trying to standardize the output of sampling rates formats across models, since we will need to standardize the key for audio watermarking to be effective: #7366, since audioseal needs to be able to get the sampling rate.
2. What is the highest-leverage variable for that outcome?
more uniformity across audio output models, broader coverage for audio watermarking once 7366 is merged, better alignment with serialization behaviors for mrv1 & v2.
3. How will this work move that variable?
Allows us to just emit the sampling rate etc as an int for models running on mrv1.
4. Whose customer or user needs does this address, and what are those needs?
Most users, since this is partially cleanup and partially in support of watermarking
Purpose
This PR is related to make the PR to make sampling rate more standardized, because currently the MRv1 integration (default) doesn't broadcast scalars. It would be nice to handle this correctly so that we can just emit an int instead of hacks like wrapping it in a 0d tensor. the sampling rate PR will touch a lot of files, so it would be nice to do this first.
Test Plan
Tests added. This code is also very much on the hot path for audio models
vLLM Version: 0.31
vLLM-Omni Commit: ebe2c18
Test Result
Tests pass, ints are handled in payload building.