[feat][SFT] Support multiple datasets for training and evaluation in SFTTrainer#1883
Draft
avigyabb wants to merge 2 commits into
Draft
[feat][SFT] Support multiple datasets for training and evaluation in SFTTrainer#1883avigyabb wants to merge 2 commits into
avigyabb wants to merge 2 commits into
Conversation
…SFTTrainer Implements RFC NovaSky-AI#1875: native SFTTrainer support for training on a weighted mixture of datasets and evaluating on multiple named datasets. - Config: add train_datasets/train_dataset_splits/train_dataset_weights and eval_datasets/eval_dataset_splits/eval_dataset_names. The singular dataset_name/dataset_split and eval_dataset_name/eval_dataset_split fields are deprecated: still accepted, translated to the list form with a DeprecationWarning, and rejected when combined with the new fields. - Sampler: promote DataMixingSampler from examples into skyrl.train.dataset.samplers. The core version draws a fresh weighted plan every epoch (the example replayed one fixed plan) and checkpoints its RNG state, so mid-epoch and epoch-boundary resumes reproduce the uninterrupted sample stream exactly. The example file re-exports the core class for backward compatibility with existing sampler_class_path configs. - Trainer: tokenize each training dataset independently (per-dataset cache keys preserved) and concatenate; with multiple datasets, sampler=random selects DataMixingSampler configured with the tokenized lengths and train_dataset_weights (per-batch source ratios, independent of dataset sizes, default 1/N). Custom samplers receive the per-dataset lengths via an injected `lengths` kwarg (user-supplied kwargs win). - Eval: one dataloader per eval dataset, each evaluated separately. Metrics are logged under eval/{name}/loss, nested even for a single eval dataset. BREAKING: the former eval/eval_loss key is now eval/{name}/loss. - Tests: CPU coverage for config normalization/validation, the core sampler (fresh plan per epoch, exact resume, old-format state), sampler dispatch, and the RFC mixing-ratio test plan (user weights, default equal mixing, size-independence). - Docs/examples: README multi-dataset section, new run_sft_megatron_multi_dataset.sh, and example/e2e scripts migrated to the list-based fields. Closes NovaSky-AI#1875 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Avi Basnet <avigyabb@stanford.edu>
Found by the GPU e2e run: OmegaConf/YAML cannot parse HF slice syntax unquoted inside a flow sequence -- train_dataset_splits=[train[:2000]] fails with a ParserError on the nested brackets. Each element must be quoted: "train_dataset_splits=['train[:2000]']". Fix all migrated run scripts, the e2e script, and the README examples, and add a CPU regression test encoding the supported CLI syntax. Verified end-to-end on 4xL40S via tests/train/gpu_e2e_test/sft_tulu3_megatron.sh: 100 steps, downward loss trend, all wandb CI assertions passed (run: sky-posttraining-uc-berkeley/skyrl_sft_ci/rii88p8v). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Avi Basnet <avigyabb@stanford.edu>
Collaborator
Author
|
Ran the GPU e2e test (
The first e2e attempt caught a real bug, fixed in fe51254: OmegaConf/YAML can't parse HF slice syntax unquoted inside a list override ( 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements RFC #1875: native
SFTTrainersupport for training on a weighted mixture of multiple datasets and evaluating on multiple named datasets, as a thin layer on top of theStatefulDataLoader/sampler infrastructure from #1842.What's in this PR
Config (
skyrl/train/config/sft_config.py)train_datasets/train_dataset_splits/train_dataset_weightsandeval_datasets/eval_dataset_splits/eval_dataset_names.dataset_name/dataset_splitandeval_dataset_name/eval_dataset_splitare deprecated: still accepted, translated to the list form with aDeprecationWarning(both CLI and programmatic paths), and rejected when combined with the new fields.1/Nequal mixing), eval names unique; explicittrain_dataset_weightsonly valid withsampler=random(custom samplers take ratios viasampler_kwargs).DataMixingSampler promoted to core (
skyrl/train/dataset/samplers.py)examples/train/sft/data_mixing_sampler.py(which now re-exports it, so existingsampler_class_pathconfigs keep working)._planonce and replayed it), andstate_dictnow includes the generator state, so mid-epoch and epoch-boundary checkpoint resumes reproduce the uninterrupted sample stream exactly. Old position-only states load best-effort with a warning.Trainer (
skyrl/train/sft_trainer.py)_load_and_tokenizepath (per-(name, split)cache keys preserved), then concatenated with per-dataset lengths tracked.sampler=randomstill uses the dataloader's built-in shuffle). Multiple datasets:sampler=randomselectsDataMixingSamplerconfigured with the tokenized lengths andtrain_dataset_weights— per-batch source ratios, independent of dataset sizes.lengthsvia an injected kwarg (user-suppliedsampler_kwargswin).eval/{name}/loss— nested even with a single eval dataset so runs with and without mixing chart the same keys.data.ptdataloader state; no new mechanism.The eval metric key
eval/eval_lossis noweval/{name}/loss(intentional per the RFC). Dashboards/callbacks keying on the old name need updating.Tests (CPU-only, per the RFC test plan)
[0.8, 0.2]→ ~80/20), default weights give equal mixing, and ratios are independent of dataset sizes (20-vs-180 sources still ~50/50).StatefulDataLoader), old-format state compatibility.test_sft_config.py; callback test updated to the nested eval keys.Docs & examples
examples/train/sft/README.md: new Multi-dataset training and evaluation section, config reference updates, breaking-change note; removed the single-dataset limitation.examples/train/sft/run_sft_megatron_multi_dataset.sh; existing SFT run scripts (and the GPU e2e script) migrated to the list fields so layeringtrain_datasets=[...]overrides on them doesn't conflict with the deprecated fields they used to set.Testing
tests/train/test_sft_dataloader.py,test_sft_config.py,test_sft_callbacks.py: 106 passed. Broader CPU suite (tests/train/+tests/backends/skyrl_train/, GPU dirs excluded): 965 passed; the 3 remaining failures are missingvllm/torchvision optional deps in the local env, unrelated to this change.Closes #1875
🤖 Generated with Claude Code