Skip to content

[StepWise] Trivial fix to avg_response_length metric#1351

Merged
CharlieFRuan merged 2 commits intomainfrom
trivial
Mar 19, 2026
Merged

[StepWise] Trivial fix to avg_response_length metric#1351
CharlieFRuan merged 2 commits intomainfrom
trivial

Conversation

@CharlieFRuan
Copy link
Copy Markdown
Member

@CharlieFRuan CharlieFRuan commented Mar 19, 2026

Fixes comment #1281 (comment)

There was no good reason to only account for is_last_step since response IDs are not cumulative in step-wise training (unlike input tokens, which are cumulative)


Open with Devin

@CharlieFRuan CharlieFRuan merged commit 11d13fc into main Mar 19, 2026
3 checks passed
@CharlieFRuan CharlieFRuan deleted the trivial branch March 19, 2026 17:28
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request correctly addresses the issue of avg_response_length calculation in step-wise training. By removing the conditional logic based on is_last_step, the metric now accurately reflects the average length of all response IDs, which are per-turn samples. The documentation has also been updated to reflect this change, improving clarity and consistency.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

devpatelio pushed a commit that referenced this pull request Mar 20, 2026
Fixes comment
#1281 (comment)

There was no good reason to only account for `is_last_step` since
response IDs are not cumulative in step-wise training (unlike input
tokens, which are cumulative)
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.

1 participant