Skip to content

feat(io): unify undecoding behavior and support overlong images list - #1626

Merged
Yunnglin merged 4 commits into
modelscope:mainfrom
Moenupa:feat/better-undecode
Aug 25, 2026
Merged

feat(io): unify undecoding behavior and support overlong images list#1626
Yunnglin merged 4 commits into
modelscope:mainfrom
Moenupa:feat/better-undecode

Conversation

@Moenupa

@Moenupa Moenupa commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • (feat) add function undecode_media in io_utils, with a no-op bypass when nothing to do. It takes an optional batch_size which defaults to env var DATASET_TF_BATCH_SIZE or 1000.
  • (break) migrate parquet_to_list to use undecode_media for binary media
  • (break) migrate remote loading RemoteLoader to use undecode_media for binary media
  • (test) add tests to:
    • 1. non-media columns will maintain and keeps origianl values.
    • 2. regression tests to show why supporting batch_size: default 1000 may fail in extreme cases.
  • (doc) update env var docs.

Tests

pytest tests/benchmark/test_general_vqa_adapter.py
pytest tests/benchmark/test_general_vmcq_adapter.py
pytest tests/api/test_io_utils.py

@Yunnglin

Copy link
Copy Markdown
Collaborator

Thanks for the improvement. Before merging, could you please validate the behavior for datasets without media columns?

undecode_media() currently always calls dataset.cast(...), even when no feature is changed. Since Dataset.cast() internally runs a full Dataset.map(), this may add an unnecessary full pass and cache write for every text-only dataset loaded by RemoteDataLoader.

Could you please:

  1. Verify the performance impact on a text-only dataset and return the original dataset when no media feature is found.
  2. Add regression tests covering both a text-only dataset and the overlong image-list case.
  3. Provide a before/after reproduction for the reported Arrow offset overflow and clarify why batch_size=100 is appropriate.

Also, please add parameter and return type annotations to undecode_media().

@Moenupa

Moenupa commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Changes since the last review:

  1. Restore default batch size to 1000; it also accepts a new env var DATASET_TF_BATCH_SIZE
  2. Add a no-op bypass, if no media columns, skip casting return directly.
  3. Add tests.

A small note:
Before this PR, casting will be done once per detected media column.
After this PR, casting will only be done once per dataset (which changes every media column).

@Moenupa

Moenupa commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@Yunnglin I suggest we default batch size to 1000, for two good reasons:

  1. Hugging Face default is 1000, going against a usual default requires special attention.
    Plus, when authors are creating a dataset, they would have to handle this 1000 error.
    This is because hf dataset.map(), dataset.filter() all use this 1000 as batchsize.
  2. This overflow hurts data transform speed and only occurs in very extreme cases. As a tradeoff we should consider the general-case scenario: 100+ images is extremely rare.

@Yunnglin Yunnglin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Verified the media undecoding changes locally, including the no-op path, Arrow offset overflow regression, and Image/Audio/Video sequence coverage. All targeted tests and CI checks pass.

@Yunnglin
Yunnglin merged commit ca73555 into modelscope:main Aug 25, 2026
3 checks passed
@Moenupa
Moenupa deleted the feat/better-undecode branch August 25, 2026 07:09
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.

2 participants