Skip to content

chore(scripts): drop ad-hoc verify harnesses, move dataset converters to datasets/ - #284

Merged
haonan3 merged 2 commits into
mainfrom
chore/scripts-no-test-tree
Jul 31, 2026
Merged

chore(scripts): drop ad-hoc verify harnesses, move dataset converters to datasets/#284
haonan3 merged 2 commits into
mainfrom
chore/scripts-no-test-tree

Conversation

@haonan3

@haonan3 haonan3 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Repo policy since #99/#267 is no unenforced test tree — yet scripts/ was quietly regrowing one: #158 left seven ep_verify/ torchrun harnesses and #210 a KL-batching regression script. This PR:

  • Deletes scripts/ep_verify/ (7 files) and scripts/verify_refl_kl_batching.py. They were verification evidence for their PRs; they remain recoverable from git history (feat(veomni-ep): expert parallelism for the VeOmni FSDP2 training backend #158 / feat(experimental): add WAN ReFL as a self-contained training package #210) if anyone needs to re-run them.

  • Moves the two dataset converters next to the data they produce, following the existing datasets/<name>/ convention (cf. datasets/pickscore/prpocess.py):

    • scripts/convert_video_r1_260k_to_unirl.pydatasets/video_r1_260k/
    • scripts/convert_daily_omni_dataset_format_to_unirl.pydatasets/daily_omni_av/

    The qwen3-omni recipes' default data_path already points into exactly these directories, so converter output now lands beside the converter.

  • Fixes a copy-paste bug in the daily-omni converter docstring (its example invoked the video-r1 script) and updates self-referencing paths.

scripts/ now holds only check_recipe_targets.py — the pre-commit/CI guard, untouched here (#283 is reworking it separately).

Related Issue

N/A

Test Plan

  • python scripts/check_recipe_targets.pycheck-recipe-targets: 2310 unirl _target_ paths resolve.
  • python -m py_compile on both moved converters → OK
  • ruff check + ruff format --check on the moved files → clean
  • git grep "scripts/convert_\|scripts/ep_verify\|scripts/verify_refl" → no references anywhere (the removed/moved files were referenced nowhere outside scripts/; no CI workflow touches them)

Compatibility / Risk

Reviewer Notes

AI-assisted (Claude Code); the submitter reviewed the full diff. Duplicate-work check: scanned all open PRs touching scripts/ (#279, #280, #283) — none performs this cleanup; the only interaction is the resurrection risk noted above.

Checklist

  • I reviewed the changed code and removed unrelated/generated artifacts.
  • I updated tests, docs, and configs where needed, or explained why not.

…into datasets/

No-unenforced-test-tree policy (#99/#267): drop the seven ep_verify/
harnesses (#158) and verify_refl_kl_batching.py (#210) -- recoverable
from git history -- and move the two dataset converters next to the
data they produce (datasets/video_r1_260k/, datasets/daily_omni_av/),
matching the datasets/<name>/ convention and the recipes' default data
paths. Also fix the daily-omni docstring example, which invoked the
video-r1 script by mistake. scripts/ now holds only the
check-recipe-targets CI guard.
@leviking98z-rgb

Copy link
Copy Markdown
Collaborator

LGTM

@haonan3
haonan3 removed request for KemingWu and celve July 31, 2026 06:08
@haonan3
haonan3 merged commit 2902817 into main Jul 31, 2026
8 checks passed
@CjhHa1
CjhHa1 deleted the chore/scripts-no-test-tree branch July 31, 2026 08:10
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