Skip to content

feat(data): blend JSONL SFT sources - #5951

Merged
yaoyu-33 merged 4 commits into
mainfrom
codex/sft-jsonl-source-blend
Sep 4, 2026
Merged

yaoyu-33 merged 4 commits into
mainfrom
codex/sft-jsonl-source-blend

Conversation

@yaoyu-33

@yaoyu-33 yaoyu-33 commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add GPTSFTDatasetConfig.per_split_data_source_manifest_path for MLM-style per-split JSONL source lists and alternating weight/path blends
  • preserve the existing direct GPTSFTDataset path when a split contains only one source
  • deterministically blend raw mmap rows by relative weight, with source-local without-replacement shuffling and repeat-on-oversampling
  • feed the resolved raw blend into offline packing once, producing one packed Parquet cache per enabled packed split
  • fingerprint builder-managed caches with source paths, weights, file size, seed, preprocessing, and packing semantics
  • document configuration, sampling semantics, offline packing, and cache ownership

Semantics

Weights are positive relative row ratios and do not need to sum to one. The default blended length is the sum of source lengths; max_train_samples overrides the runtime raw-blend length when packing is disabled. Unweighted path lists consume each source row once. A one-path split uses the existing single-source construction unchanged.

This complements #5289 but does not depend on it: this PR blends raw JSONL before creating one offline-packed artifact, whereas #5289 blends multiple already-packed Parquet sources at runtime.

Test plan

  • uv run python -m pytest -q tests/unit_tests/data/datasets/test_gpt_sft.py tests/unit_tests/data/builders/test_gpt_sft_config.py tests/unit_tests/data/builders/test_gpt_sft_builder.py tests/unit_tests/data/packing/test_offline.py tests/unit_tests/doc_consistency/test_readme_consistency.py tests/unit_tests/tutorials/test_text_dataset_tutorials.py (139 passed)
  • end-to-end two-JSONL 3:1 blend smoke verifies a 6:2 source ratio in one emitted packed Parquet artifact
  • uv run pre-commit run --all-files

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 3, 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 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 0e448c2

@claude

claude Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Light review - LGTM with one minor note.

This adds MLM-style multi-JSONL blending for GPT-SFT (per_split_data_args_path / blend_output_root, GPTSFTBlendDataset, and offline-packing integration). Correctness gates look solid: exactly-one-source validation, positive-finite weight enforcement in both the parser and the dataset, single-path fast path, and cache identity that fingerprints paths, weights, and source file size/mtime. Unit test coverage is strong and targeted.

Minor (non-blocking):

  • _parse_gpt_sft_data_blend treats any even-length entry list whose even-index items all parse as floats as weight/path pairs. A two-path blend where a path happens to be a bare numeric string (e.g. ["1", "2"]) would be silently misread as weight=1, path="2". Extremely unlikely for real JSONL paths, but a one-line docstring note that even-length lists are interpreted as alternating weight/path would help future readers.
  • The docstring on GPTSFTBlendDataset.init says omitted weights make each source contribute all of its rows once on average; this matches the length-proportional default, just slightly indirect wording.

Docs: New sections in data-preparation, packed-sequences, recipe-usage, and the tutorial README are accurate and consistent with the implementation (row-based ratios, sum-of-source-rows default size, one packed artifact per enabled split, single-path passthrough).

Suggested test cases:

  • test_default_pack_path_fingerprints_blend_weights_and_source_files
  • test_offline_packing_consumes_one_blended_raw_dataset
  • test_offline_packing_materializes_one_weighted_blend_parquet
  • test_build_gpt_sft_split_applies_max_samples_to_the_blend
  • test_config_requires_exactly_one_source
  • test_blend_config_round_trip_is_declarative_and_serializable
  • test_blend_output_root_requires_per_split_data_args_path
  • test_builder_parses_mlm_style_per_split_jsonl_blends
  • test_per_split_data_args_requires_every_enabled_split
  • test_per_split_data_args_rejects_invalid_blends
  • test_gpt_sft_blend_uses_relative_weights_and_default_total_size
  • test_gpt_sft_unweighted_blend_keeps_every_source_row_once
  • test_gpt_sft_blend_is_deterministic_and_repeats_source_local_passes
  • test_gpt_sft_blend_marks_negative_index_as_lossless_padding

No perf tests impacted.

@yaoyu-33 yaoyu-33 added area:data Dataset builders, preprocessing, and samplers feature New capabilities, enhancements, or enablement work needs-more-tests Requires additional L0 and L1 test coverage before merge needs-review PR is ready for code review and waiting on a reviewer labels Sep 3, 2026
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

yaoyu-33 commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor Author

/ok to test 1ccd8d9

@copy-pr-bot

copy-pr-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

/ok to test 1ccd8d9871c6e4c3333466438dccff1a29a3f892

@yaoyu-33, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

@yaoyu-33

yaoyu-33 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 1ccd8d9

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

yaoyu-33 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test d8cf235

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

yaoyu-33 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 358e285

@yaoyu-33

yaoyu-33 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Cluster runtime validation at commit 358e2859b3a6e6da0e12afaeba66eedd82bdd111 passed on 1x H100.

  • Three-source prompt/completion manifest: Dolly/SQuAD/GSM8K, 1500/900/600 rows with 0.5/0.3/0.2 weights; resolved blend counts matched exactly.
  • Qwen3-0.6B, seq=256, MBS=1, GBS=8, 100 optimizer steps, identical seed/checkpoint/scheduler; validation/test/checkpoint saves disabled.
  • Unpacked: 100/100 finite-loss steps, first-20 median 2.355924, last-20 median 1.794558 (-23.83%), skipped=0, NaN=0.
  • Offline packed: one 1,972-row train Parquet, 94.83% packing efficiency, no sample repetition; 100/100 finite-loss steps, first-20 median 2.138398, last-20 median 1.701103 (-20.45%), skipped=0, NaN=0.
  • Raw source hashes were unchanged; resolved configs differed only in offline-packing fields and the config-output filename.

This is a functional/loss-health smoke, not a convergence-parity or throughput claim, because packing changes token composition per physical sample.

@yaoyu-33
yaoyu-33 merged commit e0d50b8 into main Sep 4, 2026
88 checks passed
@yaoyu-33
yaoyu-33 deleted the codex/sft-jsonl-source-blend branch September 4, 2026 04:26

This branch was previously deployed

1 inactive deployment
test — 358e2859 Deployed Sep 3, 2026 by copy-pr-bot[bot] via cicd-wait-in-queue #21450
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 feature New capabilities, enhancements, or enablement work needs-more-tests Requires additional L0 and L1 test coverage before merge 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.

1 participant