fix(qwen3-omni): keep audio_seqlens on CPU in get_rope_index - #300
Closed
SakaiXue6666 wants to merge 1 commit into
Closed
fix(qwen3-omni): keep audio_seqlens on CPU in get_rope_index#300SakaiXue6666 wants to merge 1 commit into
SakaiXue6666 wants to merge 1 commit into
Conversation
get_rope_index builds position ids on CPU with torch.arange(...), and the running
counters `st` / `st_idx` accumulate each segment's length into that CPU chain.
`audio_seqlens` arrives as a CUDA tensor -- callers derive it from
`feature_attention_mask.sum(1)` -- so `audio_len` stays on CUDA and poisons the
counters, and the next segment's `st_idx += text_len` raises:
RuntimeError: Expected all tensors to be on the same device,
but found at least two devices, cuda:0 and cpu!
The first audio segment is always safe (`st` is still a plain int at that point),
so this only fires once a sequence holds two or more audio segments.
The video branches already guard this exact way with `second_per_grids[i].cpu()`;
this mirrors it for audio. Only the compute device changes: the lengths are
identical and `position_ids` is still returned on `input_ids.device`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
PR 4 — redai-infra/Relax
Branch:
SakaiXue6666:fix/rope-index-audio-seqlens-device(based on main @f361a16)Link to open the PR:
https://github.com/redai-infra/Relax/compare/main...SakaiXue6666:Relax:fix/rope-index-audio-seqlens-device?expand=1
Title
Body (paste as-is)
What
get_rope_indexnormalizesaudio_seqlensonto the CPU before use, the same way the videobranches already do with
second_per_grids[i].cpu(). Two lines, no behavior change forsingle-audio sequences.
Why
get_rope_indexbuilds position ids on CPU withtorch.arange(...), and the running countersst/st_idxaccumulate each segment's length into that CPU chain.audio_seqlensarrives as aCUDA tensor — callers derive it from
feature_attention_mask.sum(1), e.g.modeling_qwen3_omni/model.py:_get_feat_extract_output_lengthsis pure arithmetic and preserves the device, soaudio_lenstays on CUDA and contaminates the counter:
On the next segment,
st_idxcomes fromllm_pos_ids_list[-1].max() + 1(CPU) whiletext_len = min_ed - stis now CUDA, andst_idx += text_lenraises:The first audio segment is always safe — at that point
stis still a plain int, sotext_lenis an int too. This only fires once a sequence holds two or more audio segments,which is why single-audio workloads never see it.
The asymmetry is visible in the function itself: both video branches guard with
second_per_grids[video_index].cpu(), and the audio branches do not.How it is reachable
scripts/training/multimodal/run-qwen3-30B-A3B-omni-16xgpu.shpasses--multimodal-keys '{"image":"image","audio":"audio"}', whereaudiois a list-valued field —nothing constrains it to a single clip. Any sample carrying two or more audio segments takes this
path, as does multi-turn audio interaction generally.
Verification
Being upfront about what was and was not checked:
_get_feat_extract_output_lengthsreturns a tensor (not an int)when handed a tensor element, and
sttherefore becomes a tensor from the second iterationonward, inheriting whatever device
audio_seqlensis on. This is reproducible on CPU.model.py→get_rope_index→_get_feat_extract_output_lengths→ the counter update, on main @f361a16.by definition a device mismatch, so I cannot demonstrate the
RuntimeErroritself here.What I can say from experience is that we hit this exact failure — same message, same
multi-audio trigger, same first-segment-is-fine pattern — in the equivalent function in
megatron.bridge's Qwen3-Omni model, under a multi-turn audio workload (simultaneoustranslation, roughly ten audio chunks per sample). That is a different module from the vendored
copy here, so it is corroboration rather than proof; we patched this copy pre-emptively rather
than waiting to hit it again.
Happy to add a CUDA-gated regression test if you would like one — I left it out rather than ship
a test I cannot run.
Risk
Minimal.
audio_seqlensis read in exactly two places, both feeding_get_feat_extract_output_lengthsfor a length that is then used to sizetorch.arange(...)andto advance a CPU counter — both want CPU. Only the compute device changes: the lengths are
identical and
position_idsis still returned oninput_ids.device. Single-audio sequences,which is the common path today, are bit-for-bit unaffected.
Checklist
ruffline-length (119) respected; the addition is#comments, so thedocformatterhook does not apply to it