Skip to content

fix: avoid num_return_sequences prefill batch fanout#1158

Open
Vinkle-hzt wants to merge 1 commit into
alibaba:mainfrom
Vinkle-hzt:codex/fifo-num-return-sequences-batch-size
Open

fix: avoid num_return_sequences prefill batch fanout#1158
Vinkle-hzt wants to merge 1 commit into
alibaba:mainfrom
Vinkle-hzt:codex/fifo-num-return-sequences-batch-size

Conversation

@Vinkle-hzt

@Vinkle-hzt Vinkle-hzt commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Keep non-beam num_return_sequences streams at batch size 1 for the prompt/context execution step, then fan out to num_return_sequences at the first sampling step.
  • Preserve sampler fan-out by tiling sampler token/logit inputs, cum log probs, finished masks, logits processor intervals, and dispatcher success offsets by sampler batch size.
  • Keep warmup behavior intact by treating decode warmup as perf-test mode, and size return_softmax_probs storage by max batch.

Why

For FIFO scheduling, num_return_sequences should not inflate the context/model batch size before the request has produced its first sampled token. The previous GenerateStream::batchSize(0) returned num_return_sequences for non-beam requests, so StreamGroups and model input assembly treated the prompt step as a larger batch even though the prompt only needs one model execution.

@Vinkle-hzt Vinkle-hzt marked this pull request as ready for review July 6, 2026 08:00
@Vinkle-hzt Vinkle-hzt requested a review from LLLLKKKK as a code owner July 6, 2026 08:00
@Vinkle-hzt Vinkle-hzt changed the title [codex] avoid num_return_sequences prefill batch fanout fix: avoid num_return_sequences prefill batch fanout Jul 6, 2026
@LLLLKKKK

LLLLKKKK commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1158

Status: BLOCKING

Summary: P0/0 · P1/0 · P2/1 · P3/0

Non-blocking Suggestions

P2

  • 生成式推荐 / Tree logits processor 的 tiling 状态复制路径缺少集成单测覆盖 @ rtp_llm/cpp/models/logits_processor/LogitsProcessorFactory.cc:26
    • 建议:建议补一条集成单测:构造 num_return_sequences>1 且设置 combo_token_size(或 tree 相关配置)的 stream,走一遍 createLogitsProcessors → context 步 gather/dispatch → update 的 fan-out,断言 N 份 infos_/tree_infos_ 均由 context 状态正确复制且各自独立推进,从而锁定 state_batch_size 选择与 remap 组合的正确性。此为非阻塞的覆盖补强。

Checklist Violations (1 fail / 25 total)

General Principles Checklist

  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue 生成式推荐 / Tree logits processor 的 tiling 状态复制路径缺少集成单测覆盖
    LogitsProcessorFactory 新增 need_return_sequence_tiling/state_batch_size 逻辑:non-beam 且 max_batch_size > init_batch_size 时,TreeLogitsProcessor/RecommendationLogitsProcessor 直接按 max_batch_size(=N) 建满 N 份状态,并依赖去守卫后的 updateMultiSeqStatus({0,...,0}) 在 context 步做状态复制。新增的两条单测覆盖了 softmax_probs/cum_log_probs 的 tiling,但未覆盖 num_return_sequences>1 且带 combo_token_size/tree 状态时,工厂建满 N 份 + fan-out remap 的集成行为。虽有 RecommendationLogitsProcessorTest 单测了 updateMultiSeqStatus({0,0,0}) 这一构件,

Strengths

  • batch 维度语义拆分(model batch vs sampler batch)在所有落点统一,且 StreamGroups::totalSamplerBatchSizeIn/Out 的累加口径与 dispatcher/gatherer 的 per-stream 计算严格对齐,避免了索引错位与越界。
  • isSubGenerateDoneWithoutLock(i)isSubGenerateDoneWithoutLock(cur_batch) 精准修复了 context 阶段 sub_generate_status_ 尺寸变为 1 后的潜在越界读,属于必要且到位的配套修复。
  • softmax_probs_ 改用 maxBatchSize() 而非 init_batch_size 分配,顺带消除了 beam/return-seq 下缓冲区可能欠配的隐患。
  • 新增 testNumReturnSequencesTileAfterContextBatchtestNumReturnSequencesTilesSamplerInputOnly 两条单测,分别验证 stream 级 batch 尺寸(current=1/next=20/softmax numel)与端到端 sampler tiling(token_ids/logits/cum_log_probs 复制),覆盖了主要行为契约。

@LLLLKKKK LLLLKKKK enabled auto-merge (rebase) July 7, 2026 04:40
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