feat: add num_return_sequences support for rec beam search.#1289
feat: add num_return_sequences support for rec beam search.#1289DragonFive wants to merge 13 commits intojd-opensource:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the num_return_sequences parameter across the C++, Python, and Protobuf APIs, allowing beam search to return more sequences than the beam width. The implementation adds logic to Batch and SequencesGroup to expand results using top-k logprobs and updates the RecSampler for accurate logprob calculation. Feedback identifies a critical runtime bug where the beam_base_logprobs tensor must be flattened to 1-D to prevent a shape mismatch crash during batch processing. Additionally, an include path in batch.cpp requires correction to follow the project's root-relative path style guide.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the num_return_sequences parameter across the C++, C, Python, and Protobuf APIs, allowing users to specify the number of sequences returned from beam search. The feedback identifies violations of the repository style guide regarding member functions in structs and suggests logic adjustments to correctly support sequence pruning when num_return_sequences is less than the beam width.
Summary
num_return_sequencesplumbing for REC beam search across proto, pybind, c_api, and cc_apixllm_recnum_return_sequences > beam_widthTesting
git diff --checkpython3 -m py_compile xllm/pybind/llm.py xllm/pybind/params.pybatch_test.cppandrec_sampler_test.cppNotes
feat/rec-num-return-sequencesteam/main