Skip to content

fix(data): use Energon validation recipe for val split - #5727

Open
caohch-1 wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
caohch-1:fix/5626-energon-validation
Open

fix(data): use Energon validation recipe for val split#5727
caohch-1 wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
caohch-1:fix/5626-energon-validation

Conversation

@caohch-1

@caohch-1 caohch-1 commented Aug 21, 2026

Copy link
Copy Markdown

What does this PR do ?

Use Energon's validation dataset recipe for the validation split instead of constructing validation data through the training recipe.

Changelog

  • Route the training split through get_train_dataset() and the validation split through get_val_dataset().
  • Keep training-only shuffle configuration out of the validation path while preserving existing dataset-loading options.
  • Add regression coverage for validation dataset recipe selection and update existing validation dataloader tests to use the validation builder.

Validation

  • A focused regression harness confirms that the pre-fix implementation selects get_train_dataset() for validation and the updated implementation selects get_val_dataset().
  • The same harness confirms that the training split continues to use get_train_dataset().
  • git diff --check passes.
  • Python compilation checks pass for both changed files.

GitHub Actions CI

This is an external contribution, so GitHub Actions CI will require approval from an NVIDIA developer.

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation? No documentation changes are needed for this fix.
  • Does the PR affect components that are optional to install? No.
    • Reviewer: Does the PR have correct import guards for all optional libraries?

Additional Information

Fixes #5626

Signed-off-by: caohch-1 <caohch2023@shanghaitech.edu.cn>
@copy-pr-bot

copy-pr-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@yaoyu-33 yaoyu-33 added area:data Dataset builders, preprocessing, and samplers bug Something isn't working needs-review PR is ready for code review and waiting on a reviewer labels Aug 21, 2026
self.path,
batch_size=self.micro_batch_size,
task_encoder=task_encoder,
task_encoder=self.validation_task_encoder,

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.

validation_task_encoder will this always exist? or required

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes. validation_task_encoder is initialized in __init__ as validation_task_encoder or self.task_encoder, so it falls back to the training task encoder when a separate validation encoder is not provided. The existing val path already used self.validation_task_encoder; this PR only changes the dataset builder from get_train_dataset() to get_val_dataset().

If both encoders are None in direct construction, the value can still be None, but that is pre-existing behavior and is not introduced by this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:data Dataset builders, preprocessing, and samplers bug Something isn't working community-request needs-review PR is ready for code review and waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Energon validation set is shuffled and never resets — validation loss is not reproducible

2 participants