Skip to content

Commit 77eda18

Browse files
KKothuriclaude
andauthored
Add flexible data source discovery to load_raw_dataset (#661) (#675)
Resolves #661. ## Summary Extends `load_raw_dataset()` in `src/speculators/data_generation/preprocessing.py` from two source types to the four-step resolution chain requested in RFC #661, and promotes Magpie/Nemotron to shared presets. Scope is intentionally limited to **data source discovery** — schema/role normalization is left to a separate effort. ## Changes - `load_raw_dataset` now resolves a source in order: 1. Local `.json`/`.jsonl` file (existing). 2. **Local directory** — recursively glob `*.json`/`*.jsonl` (sorted for determinism) and load as one dataset; an empty directory raises an actionable error. 3. Named preset from `DATASET_CONFIGS` (existing). 4. **`hf:<id>[:<subset>:<split>]`** — load an arbitrary HuggingFace dataset. - New `_load_hf_dataset` helper parses the `hf:` spec (split defaults to `train`). It **raises if the loaded dataset has no `conversations` column**, since `_preprocess_batch` otherwise only warns and silently yields zero samples — satisfying the RFC's "no silent data corruption" requirement. - Adds `magpie` (already in ShareGPT `conversations` format, no `normalize_fn`) and `nemotron` (`nvidia/Llama-Nemotron-Post-Training-Dataset`, `SFT`/`chat`, with `_normalize_nemotron` building conversations from `input` + `output`) presets. Both schemas were verified against the live HF datasets-server before hardcoding. All source types compose across mixed `--data` usage, e.g.: ``` --data sharegpt --data /my/shards/ --data hf:nvidia/HelpSteer2 --data extra.jsonl ``` ### Note for the maintainer Beyond the RFC's literal `hf:<id>:<subset>:<split>`, I also accept the 2-part form `hf:<id>:<split>` (subset omitted), since single-config datasets with a non-`train` split (e.g. `HuggingFaceH4/ultrachat_200k:train_sft`) are common. Happy to make it strict if you'd prefer. ## Why this is not a duplicate #637 (RFC #583) touches the same function but bundles directory/HF discovery **together with** role/schema normalization. Per @shanjiaz's scoping on #583 and the assignment of #661, this PR delivers the **discovery slice only**, keeping it separate from normalization. The `hf:` handling here is also a different, more explicit design: an explicit `hf:` prefix (rather than treating any unmatched string as an HF path) plus a hard failure on non-conversations data. ## Tests New tests in `tests/integration/datagen/test_preprocessing.py`: local file, recursive directory bundle (incl. nested `.json`), empty-directory error, preset dispatch, unsupported source, `hf:` spec parsing (parametrized), the conversations-format guard, malformed `hf:` specs, and the new presets. The `hf:`/preset cases mock `load_dataset` so they are deterministic and network-free. Commands run (in the project venv): ``` python -m pytest tests/integration/datagen/test_preprocessing.py -q # 51 passed, 2 skipped ruff check src/speculators/data_generation/{preprocessing,configs}.py tests/integration/datagen/test_preprocessing.py # All checks passed ruff format --check <same files> # already formatted python -m mypy src/speculators/data_generation/{preprocessing,configs}.py # Success: no issues ``` ## AI assistance disclosure AI assistance (Claude Code) was used to help write this change. I (the submitter) have reviewed every changed line, understand the change end-to-end, and ran the tests above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Karthik Kothuri <karthikkothuri2009@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 73951eb commit 77eda18

3 files changed

Lines changed: 310 additions & 12 deletions

File tree

src/speculators/data_generation/configs.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ def _normalize_gsm8k(example: dict) -> dict:
3737
}
3838

3939

40+
def _normalize_nemotron(example: dict) -> dict:
41+
"""Build a conversation from Nemotron ``input`` turns plus ``output``."""
42+
return {
43+
"conversations": [
44+
*example["input"],
45+
{"role": "assistant", "content": example["output"]},
46+
]
47+
}
48+
49+
4050
def get_coco_dir():
4151
return os.getenv("COCO_DIR") or "coco/"
4252

@@ -110,6 +120,18 @@ def _normalize_sharegpt4v_coco(example: dict) -> dict:
110120
split="train",
111121
normalize_fn=_normalize_gsm8k,
112122
),
123+
"magpie": DatasetConfig(
124+
name="magpie",
125+
hf_path="Magpie-Align/Magpie-Llama-3.1-Pro-300K-Filtered",
126+
split="train",
127+
),
128+
"nemotron": DatasetConfig(
129+
name="nemotron",
130+
hf_path="nvidia/Llama-Nemotron-Post-Training-Dataset",
131+
subset="SFT",
132+
split="chat",
133+
normalize_fn=_normalize_nemotron,
134+
),
113135
# NOTE: You need to serve vLLM with `--allowed-local-media-path /path/to/coco`
114136
"sharegpt4v_coco": DatasetConfig(
115137
name="sharegpt4v_coco",

src/speculators/data_generation/preprocessing.py

Lines changed: 100 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -651,26 +651,114 @@ def build_eagle3_dataset(
651651
return dataset
652652

653653

654+
def _load_hf_dataset(spec: str) -> tuple[HFDataset, None]:
655+
"""Load an arbitrary HuggingFace dataset from an ``hf:`` spec.
656+
657+
Args:
658+
spec: ``hf:<dataset_id>[:<subset>:<split>]``. The split defaults to
659+
``train``. A single suffix (``hf:<id>:<split>``) selects a split
660+
without a subset; both can be given as ``hf:<id>:<subset>:<split>``.
661+
662+
Returns:
663+
Tuple of (raw_dataset, None). No normalize_fn is applied: the dataset
664+
must already be in conversations format.
665+
666+
Raises:
667+
ValueError: If the spec is malformed or the loaded dataset has no
668+
``conversations`` column.
669+
"""
670+
subset: str | None
671+
match spec.removeprefix("hf:").split(":"):
672+
case [hf_id]:
673+
subset, split = None, "train"
674+
case [hf_id, split]:
675+
subset = None
676+
case [hf_id, subset, split]:
677+
pass
678+
case _:
679+
raise ValueError(
680+
f"Invalid hf: spec '{spec}'. "
681+
f"Expected hf:<dataset_id>[:<subset>:<split>]."
682+
)
683+
684+
if not hf_id:
685+
raise ValueError(f"Invalid hf: spec '{spec}': missing dataset id.")
686+
if subset == "":
687+
raise ValueError(f"Invalid hf: spec '{spec}': empty subset.")
688+
if not split:
689+
raise ValueError(f"Invalid hf: spec '{spec}': empty split.")
690+
691+
raw_dataset = load_dataset(hf_id, name=subset, split=split)
692+
693+
if "conversations" not in raw_dataset.column_names:
694+
raise ValueError(
695+
f"HuggingFace dataset '{hf_id}' (split '{split}') is not in "
696+
f"conversations format: expected a 'conversations' column but found "
697+
f"{raw_dataset.column_names}. Pass a dataset already in conversations "
698+
f"format, or add a preset to DATASET_CONFIGS with a normalize_fn."
699+
)
700+
701+
return raw_dataset, None
702+
703+
654704
def load_raw_dataset(
655705
train_data_path: str,
656706
) -> tuple[HFDataset, Callable[[dict], dict] | None]:
657-
"""Load raw dataset from local file or HuggingFace."""
707+
"""Load a raw dataset from one of several source types.
708+
709+
Resolution order:
710+
1. Local ``.json``/``.jsonl`` file.
711+
2. Local directory: recursively load all ``*.json``/``*.jsonl`` files
712+
as a single dataset.
713+
3. Named preset from ``DATASET_CONFIGS``.
714+
4. ``hf:<id>[:<subset>:<split>]`` for an arbitrary HuggingFace dataset.
715+
716+
Args:
717+
train_data_path: File path, directory path, preset name, or ``hf:`` spec.
718+
719+
Returns:
720+
Tuple of (raw_dataset, normalize_fn). normalize_fn is None for sources
721+
already in conversations format.
722+
723+
Raises:
724+
ValueError: If the source cannot be resolved or a local directory
725+
contains no ``.json``/``.jsonl`` files.
726+
"""
727+
# 1. Local file
658728
if train_data_path.endswith((".jsonl", ".json")):
659729
return load_dataset("json", data_files=train_data_path, split="train"), None
660730

661-
if train_data_path not in DATASET_CONFIGS:
662-
raise ValueError(
663-
f"Unsupported dataset: {train_data_path}. "
664-
f"Supported: local .json/.jsonl files or {list(DATASET_CONFIGS.keys())}"
731+
# 2. Local directory
732+
path = Path(train_data_path)
733+
if path.is_dir():
734+
data_files = sorted(
735+
str(p) for p in (*path.rglob("*.json"), *path.rglob("*.jsonl"))
665736
)
737+
if not data_files:
738+
raise ValueError(
739+
f"No .json/.jsonl files found in directory: {train_data_path}"
740+
)
741+
return load_dataset("json", data_files=data_files, split="train"), None
666742

667-
config = DATASET_CONFIGS[train_data_path]
668-
raw_dataset = load_dataset(config.hf_path, name=config.subset, split=config.split)
669-
670-
if config.filter_fn is not None:
671-
raw_dataset = raw_dataset.filter(config.filter_fn)
672-
673-
return raw_dataset, config.normalize_fn
743+
# 3. Named preset
744+
if train_data_path in DATASET_CONFIGS:
745+
config = DATASET_CONFIGS[train_data_path]
746+
raw_dataset = load_dataset(
747+
config.hf_path, name=config.subset, split=config.split
748+
)
749+
if config.filter_fn is not None:
750+
raw_dataset = raw_dataset.filter(config.filter_fn)
751+
return raw_dataset, config.normalize_fn
752+
753+
# 4. Arbitrary HuggingFace dataset
754+
if train_data_path.startswith("hf:"):
755+
return _load_hf_dataset(train_data_path)
756+
757+
raise ValueError(
758+
f"Unsupported dataset: {train_data_path}. Supported: local .json/.jsonl "
759+
f"file, local directory of .json/.jsonl files, hf:<id>[:<subset>:<split>], "
760+
f"or a preset {list(DATASET_CONFIGS.keys())}."
761+
)
674762

675763

676764
def get_tokenizer(processor: ProcessorLike):

tests/integration/datagen/test_preprocessing.py

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,32 @@
55
import json
66
import re
77
from typing import Any
8+
from unittest.mock import patch
89

910
import pytest
1011
import torch
1112
from datasets import Dataset as HFDataset
1213
from PIL import Image
1314
from transformers import AutoTokenizer
1415

16+
from speculators.data_generation.configs import (
17+
DATASET_CONFIGS,
18+
_normalize_nemotron,
19+
)
1520
from speculators.data_generation.preprocessing import (
1621
_adapt_conv_for_hf,
1722
_adapt_conv_for_vllm,
1823
_create_loss_mask_from_offsets,
1924
_detect_assistant_pattern,
25+
_load_hf_dataset,
2026
_normalize_conversation,
2127
_preprocess_batch,
2228
_supports_assistant_mask,
2329
build_eagle3_dataset,
2430
get_tokenizer,
2531
load_and_preprocess_dataset,
2632
load_processor,
33+
load_raw_dataset,
2734
)
2835

2936
# Test model from HuggingFace with chat template
@@ -1397,6 +1404,187 @@ def test_normalize_conversation_tool_calls_with_empty_content():
13971404
assert assistant_tool_call_turn["tool_calls"] == tool_calls
13981405

13991406

1407+
# Tests for load_raw_dataset resolution chain (issue #661)
1408+
1409+
PREFIX = "speculators.data_generation.preprocessing"
1410+
1411+
1412+
def _write_jsonl(path, rows):
1413+
"""Write rows as newline-delimited JSON to path."""
1414+
path.write_text("\n".join(json.dumps(r) for r in rows))
1415+
1416+
1417+
def _conv_row(prompt: str) -> dict:
1418+
"""Return one conversations-format row for the given prompt."""
1419+
return {
1420+
"conversations": [
1421+
{"from": "human", "value": prompt},
1422+
{"from": "gpt", "value": f"answer to {prompt}"},
1423+
]
1424+
}
1425+
1426+
1427+
@pytest.mark.sanity
1428+
def test_load_raw_dataset_local_file(tmp_path):
1429+
"""A local .jsonl file loads with no normalize_fn."""
1430+
data_file = tmp_path / "data.jsonl"
1431+
_write_jsonl(data_file, [_conv_row("a"), _conv_row("b")])
1432+
1433+
dataset, normalize_fn = load_raw_dataset(str(data_file))
1434+
1435+
assert len(dataset) == 2
1436+
assert normalize_fn is None
1437+
1438+
1439+
@pytest.mark.sanity
1440+
def test_load_raw_dataset_local_directory(tmp_path):
1441+
"""A directory of .json/.jsonl shards loads as one combined dataset."""
1442+
_write_jsonl(tmp_path / "shard1.jsonl", [_conv_row("a"), _conv_row("b")])
1443+
_write_jsonl(tmp_path / "shard2.jsonl", [_conv_row("c")])
1444+
# Nested file is discovered recursively; .json extension also matched.
1445+
nested = tmp_path / "nested"
1446+
nested.mkdir()
1447+
_write_jsonl(nested / "shard3.json", [_conv_row("d")])
1448+
1449+
dataset, normalize_fn = load_raw_dataset(str(tmp_path))
1450+
1451+
assert len(dataset) == 4
1452+
assert normalize_fn is None
1453+
1454+
1455+
@pytest.mark.sanity
1456+
def test_load_raw_dataset_empty_directory_raises(tmp_path):
1457+
"""A directory with no .json/.jsonl files raises an actionable error."""
1458+
empty_dir = tmp_path / "empty"
1459+
empty_dir.mkdir()
1460+
1461+
with pytest.raises(ValueError, match="No .json/.jsonl files found"):
1462+
load_raw_dataset(str(empty_dir))
1463+
1464+
1465+
@pytest.mark.sanity
1466+
def test_load_raw_dataset_named_preset():
1467+
"""A named preset resolves through DATASET_CONFIGS to load_dataset."""
1468+
sentinel = HFDataset.from_list([_conv_row("x")])
1469+
with patch(f"{PREFIX}.load_dataset", return_value=sentinel) as mock_load:
1470+
dataset, normalize_fn = load_raw_dataset("sharegpt")
1471+
1472+
config = DATASET_CONFIGS["sharegpt"]
1473+
mock_load.assert_called_once_with(
1474+
config.hf_path, name=config.subset, split=config.split
1475+
)
1476+
assert dataset is sentinel
1477+
assert normalize_fn is config.normalize_fn
1478+
1479+
1480+
@pytest.mark.sanity
1481+
def test_load_raw_dataset_unsupported_source_raises():
1482+
"""An unknown source that is not a file/dir/preset/hf: spec raises."""
1483+
with pytest.raises(ValueError, match="Unsupported dataset"):
1484+
load_raw_dataset("not_a_real_preset")
1485+
1486+
1487+
@pytest.mark.sanity
1488+
@pytest.mark.parametrize(
1489+
("spec", "expected_id", "expected_name", "expected_split"),
1490+
[
1491+
("hf:org/name", "org/name", None, "train"),
1492+
("hf:org/name:custom_split", "org/name", None, "custom_split"),
1493+
("hf:org/name:subset:custom_split", "org/name", "subset", "custom_split"),
1494+
],
1495+
)
1496+
def test_load_hf_dataset_spec_parsing(spec, expected_id, expected_name, expected_split):
1497+
"""hf: specs parse into (id, subset, split) and call load_dataset."""
1498+
sentinel = HFDataset.from_list([_conv_row("x")])
1499+
with patch(f"{PREFIX}.load_dataset", return_value=sentinel) as mock_load:
1500+
dataset, normalize_fn = load_raw_dataset(spec)
1501+
1502+
mock_load.assert_called_once_with(
1503+
expected_id, name=expected_name, split=expected_split
1504+
)
1505+
assert dataset is sentinel
1506+
assert normalize_fn is None
1507+
1508+
1509+
# A small public dataset already in conversations format, used to exercise the
1510+
# real hf: download path end to end (the RFC asks for a small public dataset).
1511+
HF_CONV_DATASET = "philschmid/guanaco-sharegpt-style"
1512+
1513+
1514+
@pytest.mark.sanity
1515+
def test_load_raw_dataset_hf_real_download():
1516+
"""End-to-end hf: load of a small public conversations dataset.
1517+
1518+
Skipped when the dataset cannot be fetched, e.g. network-restricted CI
1519+
runners that only have a pre-baked model cache. The hf: parsing and the
1520+
conversations guard are covered deterministically by the mocked tests above.
1521+
"""
1522+
try:
1523+
dataset, normalize_fn = load_raw_dataset(f"hf:{HF_CONV_DATASET}")
1524+
except Exception as exc: # noqa: BLE001
1525+
pytest.skip(f"Could not fetch {HF_CONV_DATASET}: {exc}")
1526+
1527+
assert normalize_fn is None
1528+
assert "conversations" in dataset.column_names
1529+
assert len(dataset) > 0
1530+
1531+
1532+
@pytest.mark.sanity
1533+
def test_load_hf_dataset_non_conversations_raises():
1534+
"""An hf: dataset without a conversations column fails loudly."""
1535+
non_conv = HFDataset.from_list([{"prompt": "hi", "response": "there"}])
1536+
with (
1537+
patch(f"{PREFIX}.load_dataset", return_value=non_conv),
1538+
pytest.raises(ValueError, match="conversations format"),
1539+
):
1540+
_load_hf_dataset("hf:org/name")
1541+
1542+
1543+
@pytest.mark.sanity
1544+
@pytest.mark.parametrize(
1545+
"spec",
1546+
[
1547+
"hf:org/name:a:b:c", # too many segments
1548+
"hf:", # missing id
1549+
"hf:org/name:", # trailing colon -> empty split
1550+
"hf:org/name:subset:", # empty split with subset
1551+
"hf:org/name::split", # empty subset
1552+
],
1553+
)
1554+
def test_load_hf_dataset_malformed_spec_raises(spec):
1555+
"""Malformed hf: specs raise locally without touching the network."""
1556+
with (
1557+
patch(f"{PREFIX}.load_dataset") as mock_load,
1558+
pytest.raises(ValueError, match="Invalid hf: spec"),
1559+
):
1560+
_load_hf_dataset(spec)
1561+
mock_load.assert_not_called()
1562+
1563+
1564+
@pytest.mark.sanity
1565+
def test_dataset_configs_has_magpie_and_nemotron():
1566+
"""magpie and nemotron presets are registered."""
1567+
assert "magpie" in DATASET_CONFIGS
1568+
assert "nemotron" in DATASET_CONFIGS
1569+
# magpie ships conversations already, so needs no normalize_fn.
1570+
assert DATASET_CONFIGS["magpie"].normalize_fn is None
1571+
1572+
1573+
@pytest.mark.sanity
1574+
def test_normalize_nemotron_builds_conversations():
1575+
"""nemotron normalize_fn appends the output as an assistant turn."""
1576+
example = {
1577+
"input": [{"role": "user", "content": "hi"}],
1578+
"output": "hello",
1579+
}
1580+
result = _normalize_nemotron(example)
1581+
1582+
assert result["conversations"] == [
1583+
{"role": "user", "content": "hi"},
1584+
{"role": "assistant", "content": "hello"},
1585+
]
1586+
1587+
14001588
# Tests for load_and_preprocess_dataset
14011589

14021590

0 commit comments

Comments
 (0)