Skip to content

[trainer, data] fix: do not drop_last SFT validation batches - #7467

Open
YeonwooSung wants to merge 1 commit into
verl-project:mainfrom
YeonwooSung:fix/7464-sft-val-drop-last
Open

[trainer, data] fix: do not drop_last SFT validation batches#7467
YeonwooSung wants to merge 1 commit into
verl-project:mainfrom
YeonwooSung:fix/7464-sft-val-drop-last

Conversation

@YeonwooSung

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #7464.

Both SFT trainers built the validation loader with train_batch_size and drop_last=True. A val set smaller than that batch (default 256 on Ray SFT) produced zero val batches. The trainer then did torch.mean(torch.tensor([])) and logged val/loss = NaN with exit code 0.

The config comment said micro_batch_size_per_gpu was also the val batch size, but that field was never used for val. PPO already uses drop_last=False and a dedicated val batch size.

This PR:

  • Resolves val batch size as data.val_batch_size > data.micro_batch_size_per_gpu > len(val_dataset).
  • Uses drop_last=False on the val sampler and dataloader.
  • Skips logging val/loss when no val batches exist (SPMD path still all-reduces counts so ranks stay aligned).
  • Adds data.val_batch_size to sft_trainer_engine.yaml.

Not a duplicate of #7401 (epoch-boundary resume) or #467 / #6319 (train-loss NaN).

Checklist Before Starting

Test

pytest tests/trainer/test_sft_val_batch_on_cpu.py -q

The helper was also executed via a direct module load on this machine (no torch/ray in the login Python). Ruff passed on the touched files.

API and Usage Example

# optional explicit val batch; otherwise micro_batch_size_per_gpu (default 4) is used
data.val_batch_size=8
data.val_files=/path/to/val.parquet

A 200-sample val set with train_batch_size=256 now runs validation instead of logging NaN.

Design & Code Changes

resolve_sft_val_batch_size() lives in verl/trainer/sft_val_utils.py so both the SPMD and Ray SFT trainers share one rule without importing each other.

Checklist Before Submitting

  • Read the Contribute Guide.
  • Apply pre-commit checks: pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always
  • Add / Update the documentation.
  • Add unit or end-to-end test(s) to the CI workflow to cover all the code. If not feasible, explain why: CPU unit tests cover the batch-size resolver.
  • Once your PR is ready for CI, send a message in the ci-request channel in the verl Slack workspace.
  • If your PR is related to the recipe submodule, please also update the reference to the submodule commit via git submodule update --remote or cd recipe && git pull origin main.

AI assistance was used to locate the bug and draft the patch. A human author reviewed every changed line.

Fixes verl-project#7464

Co-authored-by: Grok <grok@x.ai>
Signed-off-by: YeonwooSung <neos960518@gmail.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.

[Bug][SFT] Validation uses drop_last=True and train_batch_size, which can drop the entire val set and log NaN val/loss

1 participant