Skip to content

Respect selected episodes when combining datasets - #350

Open
Doribelove wants to merge 1 commit into
Farama-Foundation:mainfrom
Doribelove:codex/combine-selected-episodes
Open

Respect selected episodes when combining datasets#350
Doribelove wants to merge 1 commit into
Farama-Foundation:mainfrom
Doribelove:codex/combine-selected-episodes

Conversation

@Doribelove

Copy link
Copy Markdown

Description

combine_datasets copies every episode from each input's backing storage, even when the input is a filtered or split MinariDataset. Splitting a four-episode dataset into two two-episode views and combining them therefore produces eight episodes. Combining a filtered view also restores the excluded episodes.

This change passes each dataset's episode_indices to MinariStorage.update_from_storage. The new optional argument preserves the existing full-copy behavior when omitted and copies supplied indices in order. Supplied iterables are materialized because storage backends can traverse indices more than once. Episode buffers are passed to the storage writer as an iterable, including when empty, so combining empty views also initializes a readable HDF5 destination.

The tests cover filtered, split, reordered and empty views on HDF5, Arrow and Parquet; persisted episode/step totals; episode payloads and reassigned IDs; and appending selections, including one-shot iterators, to existing storage. The API docstring and basic usage documentation describe the selection behavior.

Type of change

  • Bug fix (backward-compatible optional storage argument)
  • Documentation update

Validation

Base: 50c652274c43ee3085116861d6ab9f6e160c9ab4.

  • With the original implementation and the new view regressions: 12 failed, 1 passed in tests/utils/test_dataset_combine.py. All failures were episode-count assertions.
  • After the fix: 125 passed across tests/utils/test_dataset_combine.py and tests/dataset/test_minari_storage.py.
  • Additional local coverage: 426 passed across tests/data_collector, tests/dataset/test_minari_dataset.py, tests/utils/test_dataset_creation.py, tests/utils/test_get_normalized_score.py, tests/test_serialization.py and tests/test_hosting.py. This run emitted 59 warnings.
  • pre-commit run --all-files: all applicable hooks passed except the existing pyright error at tests/utils/test_dataset_creation.py:424 (NDArray passed to EpisodeBuffer.observations). The unmodified base reports the same one error and two warnings.
  • Source distribution and wheel build passed; git diff --check passed.

Environment: Ubuntu 22.04.5 x86_64, Python 3.12.14, NumPy 2.5.3, Gymnasium 1.3.0, h5py 3.16.0, PyArrow 25.0.1 and pytest 9.1.1; editable install with .[all,testing].

This is local data-handling validation. A full pytest -v run with HF_HUB_OFFLINE=1 stops during collection of the download and CLI tests, which query Hugging Face dataset metadata; the unmodified base stops at the same two modules. The three optional integration modules are skipped. Remote dataset, documentation execution and third-party integration suites have not been validated. No training or performance result is claimed.

Checklist

  • I have run pre-commit run --all-files (existing pyright failure described above).
  • I have run pytest -v and no errors are present.
  • I have commented the non-obvious storage handling.
  • I have made corresponding documentation changes.
  • All warnings from a complete pytest -v run have been resolved.
  • I have added tests that prove the fix is effective.
  • New and existing tests in the reported local scope pass (551 tests).

AI assistance: the implementation, tests and this description were prepared with Codex. The reported validation commands were executed locally.

Forward dataset episode indices to storage copying while retaining the
full-copy default. Preserve selection order and initialize empty output
storage through the writer. Cover dataset views and storage appends on
all three formats, and document the behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant