mirror of
https://github.com/huggingface/lerobot.git
synced 2026-07-24 18:26:11 +00:00
fix(datasets): enforce one parquet row group per episode in v3 data writes (#3807)
* fix(datasets): enforce one parquet row group per episode in v3 data writes LeRobot v3 data shards must hold exactly one row group per episode so a reader can fetch episode i with pq.ParquetFile(path).read_row_group(i) (a byte-range read) instead of loading the whole shard. The recording writer already does this (one write_table per episode); the aggregate and lerobot-annotate re-write paths instead concatenated many episodes and wrote them in one shot, collapsing the file to a single row group. - io_utils: add write_table_one_row_group_per_episode (one ParquetWriter, one write_table per episode — same pattern as the recording writer); to_parquet_with_hf_images embeds images then writes per-episode row groups; to_parquet_one_row_group_per_episode wraps it for plain frames - aggregate: route non-image data writes through the per-episode writer; leave the episodes-metadata parquet untouched (already one row/episode) - annotate: rewrite shards via the per-episode writer instead of a single bulk pq.write_table - tests: invariant coverage through the aggregate (image + video) and annotate paths No change to on-disk schema, paths, naming, rollover thresholds, or compression. Readers stay backward-compatible (old collapsed files load). * Update src/lerobot/datasets/io_utils.py Co-authored-by: Caroline Pascal <caroline8.pascal@gmail.com> Signed-off-by: Pepijn <138571049+pkooij@users.noreply.github.com> * Update src/lerobot/datasets/io_utils.py Co-authored-by: Caroline Pascal <caroline8.pascal@gmail.com> Signed-off-by: Pepijn <138571049+pkooij@users.noreply.github.com> * fix(datasets): correct indentation and add strict= in row-group helper The web-edited numpy version of write_table_one_row_group_per_episode had an over-indented line (IndentationError, breaking pre-commit + test collection) and a zip() without strict=. Fix both; behaviour unchanged. --------- Signed-off-by: Pepijn <138571049+pkooij@users.noreply.github.com> Co-authored-by: Caroline Pascal <caroline8.pascal@gmail.com>
This commit is contained in:
@@ -54,6 +54,7 @@ from typing import Any
|
|||||||
import pyarrow as pa
|
import pyarrow as pa
|
||||||
import pyarrow.parquet as pq
|
import pyarrow.parquet as pq
|
||||||
|
|
||||||
|
from lerobot.datasets.io_utils import write_table_one_row_group_per_episode
|
||||||
from lerobot.datasets.language import (
|
from lerobot.datasets.language import (
|
||||||
EVENT_ONLY_STYLES,
|
EVENT_ONLY_STYLES,
|
||||||
LANGUAGE_EVENTS,
|
LANGUAGE_EVENTS,
|
||||||
@@ -274,12 +275,11 @@ class LanguageColumnsWriter:
|
|||||||
new_table = self._materialize_table(
|
new_table = self._materialize_table(
|
||||||
table, per_row_persistent, per_row_events, drop_old=self.drop_existing_subtask_index
|
table, per_row_persistent, per_row_events, drop_old=self.drop_existing_subtask_index
|
||||||
)
|
)
|
||||||
# Atomic replace: write to a sibling tmp path and rename so a crash
|
# Re-emit one row group per episode (a bulk pq.write_table would collapse
|
||||||
# mid-write can't leave a half-written shard that ``pq.read_table``
|
# them into one). Write to a sibling tmp path and atomically rename so a
|
||||||
# would then fail to open. ``Path.replace`` is atomic on POSIX +
|
# crash mid-write can't leave a half-written shard.
|
||||||
# Windows when source and target sit on the same filesystem.
|
|
||||||
tmp_path = path.with_suffix(path.suffix + ".tmp")
|
tmp_path = path.with_suffix(path.suffix + ".tmp")
|
||||||
pq.write_table(new_table, tmp_path)
|
write_table_one_row_group_per_episode(new_table, tmp_path)
|
||||||
tmp_path.replace(path)
|
tmp_path.replace(path)
|
||||||
|
|
||||||
def _materialize_table(
|
def _materialize_table(
|
||||||
|
|||||||
@@ -32,6 +32,7 @@ from .feature_utils import features_equal_for_merge, get_hf_features_from_featur
|
|||||||
from .io_utils import (
|
from .io_utils import (
|
||||||
get_file_size_in_mb,
|
get_file_size_in_mb,
|
||||||
get_parquet_file_size_in_mb,
|
get_parquet_file_size_in_mb,
|
||||||
|
to_parquet_one_row_group_per_episode,
|
||||||
to_parquet_with_hf_images,
|
to_parquet_with_hf_images,
|
||||||
write_info,
|
write_info,
|
||||||
write_stats,
|
write_stats,
|
||||||
@@ -551,6 +552,7 @@ def aggregate_data(src_meta, dst_meta, data_idx, data_files_size_in_mb, chunk_si
|
|||||||
aggr_root=dst_meta.root,
|
aggr_root=dst_meta.root,
|
||||||
hf_features=hf_features,
|
hf_features=hf_features,
|
||||||
concatenate=concatenate_data,
|
concatenate=concatenate_data,
|
||||||
|
one_row_group_per_episode=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Record the mapping from source to actual destination
|
# Record the mapping from source to actual destination
|
||||||
@@ -628,6 +630,7 @@ def append_or_create_parquet_file(
|
|||||||
aggr_root: Path = None,
|
aggr_root: Path = None,
|
||||||
hf_features: datasets.Features | None = None,
|
hf_features: datasets.Features | None = None,
|
||||||
concatenate: bool = True,
|
concatenate: bool = True,
|
||||||
|
one_row_group_per_episode: bool = False,
|
||||||
) -> tuple[dict[str, int], tuple[int, int]]:
|
) -> tuple[dict[str, int], tuple[int, int]]:
|
||||||
"""Appends data to an existing parquet file or creates a new one based on size constraints.
|
"""Appends data to an existing parquet file or creates a new one based on size constraints.
|
||||||
|
|
||||||
@@ -645,6 +648,8 @@ def append_or_create_parquet_file(
|
|||||||
aggr_root: Root path for the aggregated dataset.
|
aggr_root: Root path for the aggregated dataset.
|
||||||
hf_features: Optional HuggingFace Features schema for proper image typing.
|
hf_features: Optional HuggingFace Features schema for proper image typing.
|
||||||
concatenate: When False, always rotate to a new file instead of appending to the current one.
|
concatenate: When False, always rotate to a new file instead of appending to the current one.
|
||||||
|
one_row_group_per_episode: True for DATA parquet (emit one row group per episode); False for
|
||||||
|
the episodes-metadata parquet (already one row per episode).
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
tuple: (updated_idx, (dst_chunk, dst_file)) where updated_idx is the index dict
|
tuple: (updated_idx, (dst_chunk, dst_file)) where updated_idx is the index dict
|
||||||
@@ -657,6 +662,8 @@ def append_or_create_parquet_file(
|
|||||||
dst_path.parent.mkdir(parents=True, exist_ok=True)
|
dst_path.parent.mkdir(parents=True, exist_ok=True)
|
||||||
if contains_images:
|
if contains_images:
|
||||||
to_parquet_with_hf_images(df, dst_path, features=hf_features)
|
to_parquet_with_hf_images(df, dst_path, features=hf_features)
|
||||||
|
elif one_row_group_per_episode:
|
||||||
|
to_parquet_one_row_group_per_episode(df, dst_path)
|
||||||
else:
|
else:
|
||||||
df.to_parquet(dst_path)
|
df.to_parquet(dst_path)
|
||||||
return idx, (dst_chunk, dst_file)
|
return idx, (dst_chunk, dst_file)
|
||||||
@@ -683,6 +690,8 @@ def append_or_create_parquet_file(
|
|||||||
|
|
||||||
if contains_images:
|
if contains_images:
|
||||||
to_parquet_with_hf_images(final_df, target_path, features=hf_features)
|
to_parquet_with_hf_images(final_df, target_path, features=hf_features)
|
||||||
|
elif one_row_group_per_episode:
|
||||||
|
to_parquet_one_row_group_per_episode(final_df, target_path)
|
||||||
else:
|
else:
|
||||||
final_df.to_parquet(target_path)
|
final_df.to_parquet(target_path)
|
||||||
|
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ import datasets
|
|||||||
import numpy as np
|
import numpy as np
|
||||||
import pandas
|
import pandas
|
||||||
import pandas as pd
|
import pandas as pd
|
||||||
|
import pyarrow as pa
|
||||||
import pyarrow.dataset as pa_ds
|
import pyarrow.dataset as pa_ds
|
||||||
import pyarrow.parquet as pq
|
import pyarrow.parquet as pq
|
||||||
import torch
|
import torch
|
||||||
@@ -270,21 +271,49 @@ def hf_transform_to_torch(items_dict: dict[str, list[Any]]) -> dict[str, list[to
|
|||||||
return items_dict
|
return items_dict
|
||||||
|
|
||||||
|
|
||||||
|
def write_table_one_row_group_per_episode(table: pa.Table, path: Path) -> None:
|
||||||
|
"""Write ``table`` with one parquet row group per episode (in episode order).
|
||||||
|
|
||||||
|
Keeps shards random-access friendly (``read_row_group(i)`` fetches episode i),
|
||||||
|
mirroring the recording writer. ``table`` must carry a contiguous
|
||||||
|
``episode_index`` column.
|
||||||
|
"""
|
||||||
|
episode_index = table.column("episode_index").to_numpy(zero_copy_only=False)
|
||||||
|
starts = np.concatenate(([0], np.nonzero(np.diff(episode_index))[0] + 1))
|
||||||
|
writer = pq.ParquetWriter(str(path), table.schema, compression="snappy", use_dictionary=True)
|
||||||
|
try:
|
||||||
|
for start, stop in zip(starts, np.append(starts[1:], len(episode_index)), strict=True):
|
||||||
|
writer.write_table(table.slice(start, stop - start)) # one episode -> one row group
|
||||||
|
finally:
|
||||||
|
writer.close()
|
||||||
|
|
||||||
|
|
||||||
def to_parquet_with_hf_images(
|
def to_parquet_with_hf_images(
|
||||||
df: pandas.DataFrame, path: Path, features: datasets.Features | None = None
|
df: pandas.DataFrame, path: Path, features: datasets.Features | None = None
|
||||||
) -> None:
|
) -> None:
|
||||||
"""This function correctly writes to parquet a panda DataFrame that contains images encoded by HF dataset.
|
"""Write a DataFrame with HF-encoded images to parquet, one row group per episode.
|
||||||
This way, it can be loaded by HF dataset and correctly formatted images are returned.
|
|
||||||
|
|
||||||
Args:
|
Images are embedded into the arrow table first (``ParquetWriter.write_table``
|
||||||
df: DataFrame to write to parquet.
|
does not embed external image files like ``Dataset.to_parquet`` does).
|
||||||
path: Path to write the parquet file.
|
``features`` types image columns as ``Image()`` in the parquet schema.
|
||||||
features: Optional HuggingFace Features schema. If provided, ensures image columns
|
|
||||||
are properly typed as Image() in the parquet schema.
|
|
||||||
"""
|
"""
|
||||||
# TODO(qlhoest): replace this weird synthax by `df.to_parquet(path)` only
|
|
||||||
ds = datasets.Dataset.from_dict(df.to_dict(orient="list"), features=features)
|
ds = datasets.Dataset.from_dict(df.to_dict(orient="list"), features=features)
|
||||||
ds.to_parquet(path)
|
ds = embed_images(ds)
|
||||||
|
table = ds.with_format("arrow")[:]
|
||||||
|
if "episode_index" in table.column_names:
|
||||||
|
write_table_one_row_group_per_episode(table, path)
|
||||||
|
else:
|
||||||
|
# No episode boundaries to align row groups to — keep a single write.
|
||||||
|
pq.write_table(table, str(path))
|
||||||
|
|
||||||
|
|
||||||
|
def to_parquet_one_row_group_per_episode(df: pandas.DataFrame, path: Path) -> None:
|
||||||
|
"""Write a (non-image) DataFrame to parquet with one row group per episode."""
|
||||||
|
table = pa.Table.from_pandas(df, preserve_index=False)
|
||||||
|
if "episode_index" in table.column_names:
|
||||||
|
write_table_one_row_group_per_episode(table, path)
|
||||||
|
else:
|
||||||
|
pq.write_table(table, str(path))
|
||||||
|
|
||||||
|
|
||||||
def item_to_torch(item: dict) -> dict:
|
def item_to_torch(item: dict) -> dict:
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ import pytest
|
|||||||
pytest.importorskip("datasets", reason="datasets is required (install lerobot[dataset])")
|
pytest.importorskip("datasets", reason="datasets is required (install lerobot[dataset])")
|
||||||
pytest.importorskip("pandas", reason="pandas is required (install lerobot[dataset])")
|
pytest.importorskip("pandas", reason="pandas is required (install lerobot[dataset])")
|
||||||
|
|
||||||
|
import pandas as pd # noqa: E402
|
||||||
import pyarrow.parquet as pq # noqa: E402
|
import pyarrow.parquet as pq # noqa: E402
|
||||||
|
|
||||||
from lerobot.annotations.steerable_pipeline.reader import iter_episodes # noqa: E402
|
from lerobot.annotations.steerable_pipeline.reader import iter_episodes # noqa: E402
|
||||||
@@ -344,6 +345,78 @@ def test_annotation_metadata_sync_allows_non_streaming_load(
|
|||||||
assert len(dataset) == 24
|
assert len(dataset) == 24
|
||||||
|
|
||||||
|
|
||||||
|
def _build_packed_dataset(root: Path, episode_lengths: list[int], *, fps: int = 10) -> Path:
|
||||||
|
"""Pack several episodes into a single shard (vs build_annotation_dataset's one-per-file),
|
||||||
|
so the writer's rewrite must re-emit one row group per episode instead of collapsing them."""
|
||||||
|
from lerobot.datasets.io_utils import write_tasks
|
||||||
|
from lerobot.utils.io_utils import write_json
|
||||||
|
|
||||||
|
data_dir = root / "data" / "chunk-000"
|
||||||
|
data_dir.mkdir(parents=True, exist_ok=True)
|
||||||
|
|
||||||
|
episode_index, frame_index, timestamp, task_index, subtask_index = [], [], [], [], []
|
||||||
|
for ep, length in enumerate(episode_lengths):
|
||||||
|
episode_index += [ep] * length
|
||||||
|
frame_index += list(range(length))
|
||||||
|
timestamp += [round(i / fps, 6) for i in range(length)]
|
||||||
|
task_index += [0] * length
|
||||||
|
subtask_index += [0] * length # legacy column the writer must drop
|
||||||
|
pd.DataFrame(
|
||||||
|
{
|
||||||
|
"episode_index": episode_index,
|
||||||
|
"frame_index": frame_index,
|
||||||
|
"timestamp": timestamp,
|
||||||
|
"task_index": task_index,
|
||||||
|
"subtask_index": subtask_index,
|
||||||
|
}
|
||||||
|
).to_parquet(data_dir / "file-000.parquet", index=False)
|
||||||
|
|
||||||
|
tasks_df = pd.DataFrame({"task_index": [0]}, index=pd.Index(["do the thing"], name="task"))
|
||||||
|
write_tasks(tasks_df, root)
|
||||||
|
write_json(
|
||||||
|
{"codebase_version": "v3.1", "fps": fps, "features": {}, "total_episodes": len(episode_lengths)},
|
||||||
|
root / "meta" / "info.json",
|
||||||
|
)
|
||||||
|
return root
|
||||||
|
|
||||||
|
|
||||||
|
def test_writer_one_row_group_per_episode(tmp_path: Path) -> None:
|
||||||
|
"""Rewriting a packed shard must keep one row group per episode, not collapse
|
||||||
|
every episode into a single giant row group."""
|
||||||
|
episode_lengths = [4, 6, 5] # unequal lengths, all in one shard
|
||||||
|
root = _build_packed_dataset(tmp_path / "ds", episode_lengths)
|
||||||
|
shard = root / "data" / "chunk-000" / "file-000.parquet"
|
||||||
|
assert pq.ParquetFile(shard).metadata.num_row_groups == 1, "fixture should start collapsed"
|
||||||
|
|
||||||
|
staging_dir = tmp_path / "stage"
|
||||||
|
for ep in range(len(episode_lengths)):
|
||||||
|
_stage_episode(
|
||||||
|
staging_dir,
|
||||||
|
ep,
|
||||||
|
plan=[
|
||||||
|
{
|
||||||
|
"role": "assistant",
|
||||||
|
"content": f"subtask for ep {ep}",
|
||||||
|
"style": "subtask",
|
||||||
|
"timestamp": 0.0,
|
||||||
|
"tool_calls": None,
|
||||||
|
}
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
records = list(iter_episodes(root))
|
||||||
|
LanguageColumnsWriter().write_all(records, staging_dir, root)
|
||||||
|
|
||||||
|
# One row group per episode, with row counts matching the episode lengths.
|
||||||
|
md = pq.ParquetFile(shard).metadata
|
||||||
|
assert md.num_row_groups == len(episode_lengths)
|
||||||
|
assert [md.row_group(i).num_rows for i in range(md.num_row_groups)] == episode_lengths
|
||||||
|
# Language columns are still present after the per-episode rewrite.
|
||||||
|
table = pq.read_table(shard)
|
||||||
|
assert "language_persistent" in table.column_names
|
||||||
|
assert "language_events" in table.column_names
|
||||||
|
|
||||||
|
|
||||||
def test_speech_atom_shape_matches_plan_spec() -> None:
|
def test_speech_atom_shape_matches_plan_spec() -> None:
|
||||||
atom = speech_atom(2.5, "I'm cleaning up!")
|
atom = speech_atom(2.5, "I'm cleaning up!")
|
||||||
assert atom["role"] == "assistant"
|
assert atom["role"] == "assistant"
|
||||||
|
|||||||
@@ -32,6 +32,26 @@ from lerobot.datasets.lerobot_dataset import LeRobotDataset
|
|||||||
from tests.fixtures.constants import DUMMY_REPO_ID
|
from tests.fixtures.constants import DUMMY_REPO_ID
|
||||||
|
|
||||||
|
|
||||||
|
def assert_data_shards_one_row_group_per_episode(root):
|
||||||
|
"""Every aggregated DATA shard must have exactly one parquet row group per episode."""
|
||||||
|
import pyarrow.parquet as pq
|
||||||
|
|
||||||
|
shards = sorted((root / "data").rglob("*.parquet"))
|
||||||
|
assert shards, f"no data shards found under {root}/data"
|
||||||
|
n_episodes = 0
|
||||||
|
for shard in shards:
|
||||||
|
pf = pq.ParquetFile(shard)
|
||||||
|
episodes = pf.read(columns=["episode_index"]).column("episode_index").to_pylist()
|
||||||
|
assert pf.metadata.num_row_groups == len(set(episodes)), shard
|
||||||
|
for i in range(pf.metadata.num_row_groups):
|
||||||
|
rg_episodes = set(
|
||||||
|
pf.read_row_group(i, columns=["episode_index"]).column("episode_index").to_pylist()
|
||||||
|
)
|
||||||
|
assert len(rg_episodes) == 1, f"{shard} row group {i} spans episodes {rg_episodes}"
|
||||||
|
n_episodes += len(set(episodes))
|
||||||
|
return n_episodes
|
||||||
|
|
||||||
|
|
||||||
def assert_episode_and_frame_counts(aggr_ds, expected_episodes, expected_frames):
|
def assert_episode_and_frame_counts(aggr_ds, expected_episodes, expected_frames):
|
||||||
"""Test that total number of episodes and frames are correctly aggregated."""
|
"""Test that total number of episodes and frames are correctly aggregated."""
|
||||||
assert aggr_ds.num_episodes == expected_episodes, (
|
assert aggr_ds.num_episodes == expected_episodes, (
|
||||||
@@ -566,6 +586,41 @@ def assert_image_frames_integrity(aggr_ds, ds_0, ds_1):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("use_videos", [True, False], ids=["video", "image"])
|
||||||
|
def test_aggregate_one_row_group_per_episode(tmp_path, lerobot_dataset_factory, use_videos):
|
||||||
|
"""Aggregated DATA shards keep one row group per episode (not one collapsed group).
|
||||||
|
|
||||||
|
Covers both the non-image (``df.to_parquet``) and image
|
||||||
|
(``to_parquet_with_hf_images``) write branches, including the merge-into-
|
||||||
|
existing-file branch via a low file-size threshold that forces packing.
|
||||||
|
"""
|
||||||
|
ds_0 = lerobot_dataset_factory(
|
||||||
|
root=tmp_path / "rg_0",
|
||||||
|
repo_id=f"{DUMMY_REPO_ID}_rg_0",
|
||||||
|
total_episodes=3,
|
||||||
|
total_frames=60,
|
||||||
|
use_videos=use_videos,
|
||||||
|
)
|
||||||
|
ds_1 = lerobot_dataset_factory(
|
||||||
|
root=tmp_path / "rg_1",
|
||||||
|
repo_id=f"{DUMMY_REPO_ID}_rg_1",
|
||||||
|
total_episodes=4,
|
||||||
|
total_frames=80,
|
||||||
|
use_videos=use_videos,
|
||||||
|
)
|
||||||
|
|
||||||
|
aggr_root = tmp_path / "rg_aggr"
|
||||||
|
aggregate_datasets(
|
||||||
|
repo_ids=[ds_0.repo_id, ds_1.repo_id],
|
||||||
|
roots=[ds_0.root, ds_1.root],
|
||||||
|
aggr_repo_id=f"{DUMMY_REPO_ID}_rg_aggr",
|
||||||
|
aggr_root=aggr_root,
|
||||||
|
)
|
||||||
|
|
||||||
|
n_episodes = assert_data_shards_one_row_group_per_episode(aggr_root)
|
||||||
|
assert n_episodes == ds_0.num_episodes + ds_1.num_episodes
|
||||||
|
|
||||||
|
|
||||||
def test_aggregate_image_datasets(tmp_path, lerobot_dataset_factory):
|
def test_aggregate_image_datasets(tmp_path, lerobot_dataset_factory):
|
||||||
"""Test aggregation of image-based datasets preserves HuggingFace Image schema.
|
"""Test aggregation of image-based datasets preserves HuggingFace Image schema.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user