Skip to content

Fix max_out_len handling for multi-turn ShareGPT conversations #243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Shadowless-ly wants to merge 1 commit into
base: master
Choose a base branch
from
+8 −8
Open

Fix max_out_len handling for multi-turn ShareGPT conversations #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ais_bench/benchmark/configs/datasets/sharegpt/sharegpt_gen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@


sharegpt_reader_cfg = dict(
input_columns=["question", "answer"],
input_columns=["question", "answer", "max_out_len"],
output_column="answer"
)

Expand Down
2 changes: 1 addition & 1 deletion ais_bench/benchmark/datasets/sharegpt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def load(path, disable_shuffle, **kwargs):
)
except:
output_len = None
chat["max_out_len"] = output_len
chat["max_out_len"].append(output_len)

@wenba0 wenba0 Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这涉及到对官方数据集的接口更改，不具备通用性，可单独实现一个自定义的多轮对话数据集，兼容对max_out_len字段的实现，并在资料中体现

chat["answer"].append(data["conversations"][i + 1]["value"])
new_dataset.append(chat)
logger.info(
Expand Down
12 changes: 6 additions & 6 deletions ais_bench/benchmark/openicl/icl_inferencer/icl_multiturn_inferencer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ async def infer_last(self, data: dict, session: aiohttp.ClientSession):
output = RequestOutput(self.perf_mode)
output.turn_id, output.uuid = turn_id, uid

max_out_len = max_out_len[-1] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = max_out_len[-1] if isinstance(max_out_len, list) else max_out_len

@gemini-code-assist gemini-code-assist Bot Apr 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current logic for determining cur_max_out_len can be improved to handle cases where the max_out_len list is empty or contains None values (which can occur if tokenizer encoding fails during dataset loading). Adding a fallback to self.model.max_out_len ensures the inferencer remains robust.

Suggested change
cur_max_out_len = max_out_len[-1] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = (max_out_len[-1] if isinstance(max_out_len, list) and max_out_len else max_out_len) or self.model.max_out_len

@gemini-code-assist gemini-code-assist Bot Apr 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When max_out_len is a list, its elements (e.g., max_out_len[-1]) might be None if the tokenizer failed during dataset loading. Passing None to model.generate could cause errors. Adding a fallback to self.model.max_out_len ensures robustness.

Suggested change
cur_max_out_len = max_out_len[-1] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = (max_out_len[-1] if isinstance(max_out_len, list) and max_out_len else max_out_len) or self.model.max_out_len

await self.status_counter.post()
await self.model.generate(history, max_out_len, output, session=session, **data)
await self.model.generate(history, cur_max_out_len, output, session=session, **data)
if output.success:
await self.status_counter.rev()
else:
Expand Down Expand Up @@ -140,9 +140,9 @@ async def infer_every(self, data: dict, session: aiohttp.ClientSession):
history = await asyncio.to_thread(self.model.parse_template, PromptList(left_prompt + chat[:i] + right_prompt), mode="gen")
output = RequestOutput(self.perf_mode)
output.turn_id, output.uuid = turn_id, uid
max_out_len = max_out_len[turn_id] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = max_out_len[turn_id] if isinstance(max_out_len, list) else max_out_len

@gemini-code-assist gemini-code-assist Bot Apr 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While using cur_max_out_len correctly fixes the variable shadowing bug, the logic is still susceptible to an IndexError if the max_out_len list is shorter than the number of turns, or to None values within the list. Providing a fallback to self.model.max_out_len makes the multi-turn inference more resilient.

Suggested change
cur_max_out_len = max_out_len[turn_id] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = (max_out_len[turn_id] if isinstance(max_out_len, list) and turn_id < len(max_out_len) else (None if isinstance(max_out_len, list) else max_out_len)) or self.model.max_out_len

@gemini-code-assist gemini-code-assist Bot Apr 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to infer_last, individual elements in the max_out_len list might be None. Additionally, it's safer to check if turn_id is within the bounds of the list to avoid an IndexError if the dataset structure doesn't perfectly match the expected number of turns.

Suggested change
cur_max_out_len = max_out_len[turn_id] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = (max_out_len[turn_id] if isinstance(max_out_len, list) and turn_id < len(max_out_len) else max_out_len) or self.model.max_out_len

await self.status_counter.post()
await self.model.generate(history, max_out_len, output, session=session, **data)
await self.model.generate(history, cur_max_out_len, output, session=session, **data)
turn_id += 1
if output.success:
await self.status_counter.rev()
Expand Down Expand Up @@ -180,9 +180,9 @@ async def infer_every_with_gt(self, data: dict, session: aiohttp.ClientSession):
history = await asyncio.to_thread(self.model.parse_template, PromptList(left_prompt + chat[:i] + right_prompt), mode="gen")
output = RequestOutput(self.perf_mode)
output.turn_id, output.uuid = turn_id, uid
max_out_len = max_out_len[turn_id] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = max_out_len[turn_id] if isinstance(max_out_len, list) else max_out_len

@gemini-code-assist gemini-code-assist Bot Apr 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the infer_every method, this logic should account for potential IndexError or None values in the max_out_len list to ensure stable execution across different datasets.

Suggested change
cur_max_out_len = max_out_len[turn_id] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = (max_out_len[turn_id] if isinstance(max_out_len, list) and turn_id < len(max_out_len) else (None if isinstance(max_out_len, list) else max_out_len)) or self.model.max_out_len

@gemini-code-assist gemini-code-assist Bot Apr 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to infer_every, individual elements in the max_out_len list might be None. Adding a fallback to self.model.max_out_len and an index check ensures robustness.

Suggested change
cur_max_out_len = max_out_len[turn_id] if isinstance(max_out_len, list) else max_out_len
cur_max_out_len = (max_out_len[turn_id] if isinstance(max_out_len, list) and turn_id < len(max_out_len) else max_out_len) or self.model.max_out_len

await self.status_counter.post()
await self.model.generate(history, max_out_len, output, session=session, **data)
await self.model.generate(history, cur_max_out_len, output, session=session, **data)
turn_id += 1
if output.success:
await self.status_counter.rev()
Expand Down
Loading