Repository navigation
Conversation
The `_serialize_tensor` path called `t_cpu.numpy().tobytes()` which raises `TypeError: Got unsupported ScalarType BFloat16` for bfloat16 tensors. This crashed CosyVoice3 inter-stage communication where hidden states flow as bfloat16 (see vllm-project#5739). Store bfloat16 tensor bytes as float32 but keep the real dtype name in `tensor_dtype`, and have `_deserialize_tensor` decode them as float32 and cast back to bfloat16. Other dtypes are unchanged. Add a regression test that round-trips a bfloat16 tensor and asserts the dtype, shape and values are preserved. Fixes vllm-project#5739. Signed-off-by: panming <1925642328@qq.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Purpose Fix _serialize_tensor raising TypeError: Got unsupported ScalarType BFloat16 , which crashed CosyVoice3 inter-stage communication (#5739). numpy has no native bfloat16 , so bfloat16 tensors are now stored as float32 bytes while keeping "bfloat16" in tensor_dtype ; _deserialize_tensor decodes them as float32 and casts back to bfloat16 . Other dtypes are unchanged. Test Plan Added test_bfloat16_round_trip_preserves_values and extended test_tensor_dtype_preserved to include bfloat16 . Run: Test Result bfloat16 round-trips correctly through serialize_payload / deserialize_payload : dtype ( bfloat16 ), shape and exact values ( torch.equal == True ) all preserved. Other dtypes ( float16 , float32 , int64 , int32 , bool ) unchanged and passing. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
Omni ReviewBot: no human activity for 8 days@xiny-spec this pull request has had no human commit, comment or review since 2026-08-31. Per repository policy it may be closed if it stays inactive. To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline. |
| # :func:`_deserialize_tensor` restore the ``bfloat16`` dtype on decode. | ||
| dtype_name = _dtype_to_name(t_cpu.dtype) | ||
| if t_cpu.dtype == torch.bfloat16: | ||
| t_cpu = t_cpu.to(dtype=torch.float32) |
There was a problem hiding this comment.
Use uint16 byte views for bfloat16; skip astype for already matching NumPy dtypes.
Omni ReviewBot: no human activity for 7 days@xiny-spec this pull request has had no human commit, comment or review since 2026-09-16. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state. To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline. |
Omni ReviewBot: no human activity for 14 days@xiny-spec this pull request has had no human commit, comment or review since 2026-09-16. Please consider marking this PR as draft until work can resume. The author or a maintainer decides whether to change the PR state. To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline. |
Omni ReviewBot routing recordAssigned Strict on cursor (cursor-grok-4.6-high) under experiment |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
2 actionable finding(s).
CI at
6cd7a8ed2fd0(2026-10-09T10:29:59.602701+00:00): required check(s) blocking:pre-commit(missing),buildkite/vllm-omni(missing).
Full review analysis
Scan:
| Category | Result |
|---|---|
| Tests / verification | 1 finding(s) below |
| Security | no finding reported |
| Docs / comments | no finding reported |
| Behavior / compatibility | no finding reported |
| Correctness | 1 finding(s) below |
Validated:
- [resolved] TypeError Got unsupported ScalarType BFloat16: t_cpu.to(dtype=torch.float32) before numpy() at data_entry_keys.py:360-361. Residual: encoding is f32 bytes not bf16 bits; CosyVoice3 forked decoder still treats tensor_dtype as numpy dtype of the buffer.
- [resolved] lishunyang12 uint16/skip-astype: still unfixed on this head (f32 widen at :361 and astype at :366); main already implemented the uint8 equivalent. Residual: this PR must not land on top of that.
- [claim-verified] "other dtypes unchanged" at the tensor_dtype name/layout for float16/float32/int64/int32/bool; test_tensor_dtype_preserved now includes bfloat16. Residual: all dtypes now go through .astype(np_dtype) (already raised by lishunyang12).
- [claim-verified] numpy() raises on bf16: torch docs + video_api_utils.py:316-318 already document it.
- [claim-verified] other dtypes still go through numpy().astype; bytes unchanged if dtype already matches. Extra copy only.
- [claim-refuted] Fixes #5739 for CosyVoice3 async_chunk: private decoder at stage_input_processors/cosyvoice3.py:114 still np.dtype(dtype_name).
2 actionable finding(s).
Verdict: REQUEST CHANGES
Findings
- [P1] This PR stores bfloat16 as float32 bytes while keeping tensor_dtype='bfloat16'… —
vllm_omni/model_executor/stage_input_processors/cosyvoice3.py
Evidence for This PR stores bfloat16 as float32 bytes while keeping tensor_dtype='bfloat16'…
This PR stores bfloat16 as float32 bytes while keeping tensor_dtype='bfloat16' (data_entry_keys.py:359-368). CosyVoice3 _decode_additional_information still does dt = np.dtype(dtype_name) with no cast-back (cosyvoice3.py:114), so an AdditionalInformationPayload carrying the new tag raises (numpy has no bfloat16) or frombuffers 4-byte cells as 2-byte width and fails reshape. Default CosyVoice3 async_chunk: true (deploy/cosyvoice3.yaml:26) always runs this decoder on first-chunk init (cosyvoice3.py:152); engine transport already serializes via serialize_additional_information (orchestrator.py:139-141). glm_tts.py:252 already calls deserialize_additional_information. Existing CosyVoice3 tests pass a nested dict and short-circuit (test_cosyvoice3_stage_input_processors.py:125-131). Replace the fork with the shared helper and add a Payload+bfloat16 case (see test_ar_to_dit_async_chunk_restores_serialized_conditioning_payload).
Evidence: This PR's _serialize_tensor keeps the logical dtype after widening bf16 to float32 bytes: vllm_omni/data_entry_keys.py:359 dtype_name = _dtype_to_name(t_cpu.dtype); :360-361 if t_cpu.dtype == torch.bfloat16: / t_cpu = t_cpu.to(dtype=torch.float32); :368 tensor_dtype=dtype_name. Shared decode was updated in the same hunk: data_entry_keys.py:375-376 if entry.tensor_dtype == "bfloat16": / dt = np.dtype("float32"); :382 return tensor.to(dtype=torch.bfloat16) if entry.tensor_dtype == "bfloat16" else tensor. Unchanged by this diff, present in the PR-time tree: CosyVoice3 still does vllm_omni/model_executor/stage_input_processors/cosyvoice3.py:114 dt = np.dtype(dtype_name) with no cast-back, and default async_chunk always invokes it — vllm_omni/deploy/cosyvoice3.yaml:26 async_chunk: true; cosyvoice3.py:152 info = _decode_additional_information(getattr(request, "additional_information", None)). Engine transport already serializes to AdditionalInformationPayload (unchanged vllm_omni/engine/orchestrator.py:139-141 additional_info_payload = serialize_additional_information(). Trigger: a CosyVoice3 async_chunk first-chunk decode of a Payload whose entries were produced by the new _serialize_tensor (tensor_dtype='bfloat16', float32 bytes). Adverse effect: np.dtype("bfloat16") raises, or frombuffer treats 4-byte cells as 2-byte width and reshape fails, so streaming prompt conditioning never restores. Sibling already uses the shared helper (unchanged glm_tts.py:252 info = deserialize_additional_information(getattr(request, "additional_information", None))); CosyVoice3 tests pass a nested dict and miss the branch (unchanged tests/model_executor/stage_input_processors/test_cosyvoice3_stage_input_processors.py:125-126 additional_information={ / "embed": {).
Suggestion: def _decode_additional_information(raw_info: Any) -> dict[str, Any]:
from vllm_omni.engine.serialization import deserialize_additional_information
if raw_info is None:
return {}
if isinstance(raw_info, dict):
return raw_info
return deserialize_additional_information(raw_info)
🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!
| # as ``float32`` but keep the real dtype name in ``tensor_dtype``, and let | ||
| # :func:`_deserialize_tensor` restore the ``bfloat16`` dtype on decode. | ||
| dtype_name = _dtype_to_name(t_cpu.dtype) | ||
| if t_cpu.dtype == torch.bfloat16: |
There was a problem hiding this comment.
[P0] PR 5755 _serialize_tensor still special-cases bf16 as float32 bytes while wri…
Evidence and suggested fix
PR 5755 _serialize_tensor still special-cases bf16 as float32 bytes while writing tensor_dtype='bfloat16' (data_entry_keys.py:360-368). Current main already serializes every dtype as native bytes via t_cpu.flatten().view(torch.uint8).numpy().tobytes() and restores with torch.frombuffer(..., uint8).view(dtype) — the same pattern as in-tree serialization.py:92. Rebase onto main and drop this widen; taking the PR side would replace that general 2-byte-per-element codec with a bf16-only 2×-size float32 path for tensors such as AR payload["hidden"] (gpu_ar_model_runner.py:967), which this PR itself cites as the CosyVoice3/#5739 victim.
Evidence: Current trigger: serialize_payload on this head converts bf16 to float32 bytes but labels them bfloat16. vllm_omni/data_entry_keys.py:360-361 if t_cpu.dtype == torch.bfloat16: / t_cpu = t_cpu.to(dtype=torch.float32); :366-368 tensor_data=t_cpu.numpy().astype(np_dtype).tobytes() / tensor_dtype=dtype_name. Adverse effect if this hunk is taken over current main: bf16 wire size doubles (2B→4B/elem) and main's general codec is replaced. Unchanged by this diff, present in current main: vllm_omni/data_entry_keys.py _serialize_tensor tensor_data=t_cpu.flatten().view(torch.uint8).numpy().tobytes() and _deserialize_tensor data = torch.frombuffer(bytearray(entry.tensor_data), dtype=torch.uint8) then return data.view(dtype).reshape(entry.tensor_shape). Unchanged by this diff, present in the PR-time tree: vllm_omni/distributed/omni_connectors/utils/serialization.py:92 t = t.view(torch.uint8); vllm_omni/worker/gpu_ar_model_runner.py:967 payload["hidden"] = req_hidden_states.
Suggestion: t_cpu = t.detach().to("cpu").contiguous()
return AdditionalInformationEntry(
tensor_data=t_cpu.flatten().view(torch.uint8).numpy().tobytes(),
tensor_shape=list(t_cpu.shape),
tensor_dtype=_dtype_to_name(t_cpu.dtype),
)
Omni ReviewBot: finding feedback[p0] PR 5755 See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
Omni ReviewBot: finding feedback[p1] This PR stores bfloat16 as float32 bytes while keeping tensor_dtype='bfloat16'… — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
|
@xiny-spec Are you planning to continue work on this PR? Please reply by 2026-10-19 10:40:21 UTC. A yes or a brief plan is enough; code changes do not need to be finished within these three days. If you are waiting on a review or other maintainer action, please let us know. If there is no reply, new commit, or maintainer takeover by then, we will recheck and close this PR as inactive. It can be reopened when work resumes. |
The
_serialize_tensorpath calledt_cpu.numpy().tobytes()which raisesTypeError: Got unsupported ScalarType BFloat16for bfloat16 tensors. This crashed CosyVoice3 inter-stage communication where hidden states flow as bfloat16 (see #5739).Store bfloat16 tensor bytes as float32 but keep the real dtype name in
tensor_dtype, and have_deserialize_tensordecode them as float32 and cast back to bfloat16. Other dtypes are unchanged.Add a regression test that round-trips a bfloat16 tensor and asserts the dtype, shape and values are preserved. Fixes #5739.
PLEASE FILL IN THE PR DESCRIPTION HERE.
Purpose
Test Plan
vLLM Version:
vLLM-Omni Commit:
Test Result
BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
(anything written below this line will be removed by GitHub Actions)