Skip to content

[qoc] Extract pad_batch() into a helper to training_batch.py#1527

Merged
CharlieFRuan merged 2 commits intoNovaSky-AI:mainfrom
CharlieFRuan:extract-pad-batch
Apr 17, 2026
Merged

[qoc] Extract pad_batch() into a helper to training_batch.py#1527
CharlieFRuan merged 2 commits intoNovaSky-AI:mainfrom
CharlieFRuan:extract-pad-batch

Conversation

@CharlieFRuan
Copy link
Copy Markdown
Member

@CharlieFRuan CharlieFRuan commented Apr 17, 2026

In preparation for merging in #1483, where we will pad at mini batch level, we extract pad_batch() into a helper function in training_batch.py that is TrainingInputBatch-in and TrainingInputBatch-out.

Note that for trainer.py, pad_size > 0 only when step-wise training as of now.

The tinker engine codepath also has the same logic, where now can be de-duplicated.

Added unit tests for this method.


Open with Devin

gemini-code-assist[bot]

This comment was marked as resolved.

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 5 additional findings.

Open in Devin Review

@CharlieFRuan CharlieFRuan merged commit 9991ee3 into NovaSky-AI:main Apr 17, 2026
5 of 6 checks passed
@CharlieFRuan CharlieFRuan deleted the extract-pad-batch branch April 17, 2026 02:27
CharlieFRuan added a commit that referenced this pull request Apr 19, 2026
Rebase PR #1479 onto current main (post-PRs #1507/#1526/#1527/#1529).
The original E2E fix's `pad_batch` change is dropped since #1529's
prompt-based mini-batch boundaries removed the need to pad to
`mini_batch_size * n_samples`.

- merge_stepwise_output() in trainer_utils.py collapses multi-turn
  step-wise GeneratorOutput sequences into single sequences when
  consecutive turns share a common prefix, reducing training cost
  from O(T^2) to O(T).
- trainer.py: call merge before extracting generator fields, update
  uids from merged trajectory_ids, emit generate/num_seq_{before,after}_merge.
- Add generator.merge_stepwise_output config flag.
- run_search.sh: MERGE_STEPWISE env var.
- 16 CPU-only tests covering all 3 merging cases, partial merges, and
  validation asserts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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