Skip to content

Fix paibench_c shard task counting and enforce evaluation coverage - #377

Closed
JIN-strong wants to merge 2 commits into
NVIDIA:mainfrom
JIN-strong:fix/paibench-c-shard-count
Closed

JIN-strong wants to merge 2 commits into
NVIDIA:mainfrom
JIN-strong:fix/paibench-c-shard-count

Conversation

@JIN-strong

Copy link
Copy Markdown

What

The PAI-Bench-C runner (run_paibench_c.sh) wrote its shard JSONLs without a trailing newline and counted pending tasks with wc -l, which counts newline characters — a shard holding exactly one pending row counted as zero. The documented 4-task quick start therefore launched no workers and produced no videos, and a resumed 600-task run silently dropped the final task on every GPU left with a single-row shard.

This PR fixes the counting and closes the adjacent coverage gap:

  • input.jsonl and the per-GPU shard JSONLs are now written with a trailing newline, and pending rows are counted with awk 'END{print NR}', which is correct for unterminated and empty files alike (and, unlike wc -l, exits 0 under set -o pipefail).
  • The worker-pid wait loop uses the ${var+...} expansion so a fully-complete resume cannot abort under set -u on bash < 4.4.
  • The per-modality evaluation step is wrapped in evaluate_modality() and gated on the requested task set: the collected video count must match the row count of that modality's input.jsonl (falling back to PAIBENCH_C_NUM_SAMPLES for pre-existing trees), and any mismatch aborts with both numbers and the recovery paths instead of scoring a subset as the benchmark result.

Why

A run interrupted near the end could report metrics computed over fewer tasks than requested with no signal, and the documented default invocation could not produce any videos at all.

Fixes #376.

Validation

  • bash test_run_paibench_c.sh → 16 assertions pass. The new test extracts the real build_jsonl_and_run and evaluate_modality functions from the script and drives them with a stub inference entrypoint (no GPU, network, or venv needed): before the fix it reproduces the zero-video quick start, and it now covers single-row shard counting, fully-complete resume under set -euo pipefail, full-coverage evaluation, and rejection of a tree with one video missing (asserted to fail before any evaluation work starts).
  • bash -n on both scripts → clean.
  • shellcheck reports only warnings on pre-existing lines unrelated to this change.

The shard JSONLs and input.jsonl were written without a trailing newline and pending tasks were counted with wc -l, which counts newline characters, so a shard holding exactly one pending row counted as zero. The documented 4-task quick start interleaves one task per GPU shard and therefore generated no videos at all, and a resumed 600-task run silently dropped the final task on every GPU whose shard was left with a single row.

Write every JSONL with a trailing newline and count pending rows with awk END{NR}, which is correct for unterminated and empty files alike. Also guard the worker-pid loop expansion so a fully-complete resume cannot abort under set -u on bash < 4.4.

Add test_run_paibench_c.sh, which extracts the real build_jsonl_and_run function from the script and drives it with a stub inference entrypoint; it reproduces the zero-video quick start before the fix and passes afterwards.

Signed-off-by: JIN-strong <1228766517@qq.com>
Evaluation ran on whatever videos happened to exist under the output tree and only failed when none were present, so a run interrupted near the end (or one whose final shard tasks were dropped) computed metrics over the subset and printed them as the benchmark result with no signal.

Wrap the per-modality evaluation step in a function and gate it on the requested task set: the collected video count must match the row count of that modality's input.jsonl (falling back to PAIBENCH_C_NUM_SAMPLES for trees built before this file existed), and any mismatch aborts with both numbers and the recovery paths. Document the resume and coverage behavior in the README.

Extend test_run_paibench_c.sh to drive the real evaluate_modality function with a stub torchrun: full coverage evaluates and reports metrics, while a missing video is rejected before any evaluation work starts.

Signed-off-by: JIN-strong <1228766517@qq.com>
@yaoxu-crypto

Copy link
Copy Markdown
Collaborator

Thanks @JIN-strong for the careful fix and tests. Closing here because the evaluation suites moved to Cosmos Framework (.agents/skills/evaluation/cosmos3/generator/paibench_c/ in NVIDIA/cosmos-framework) and were removed from this repo in #384, so this PR can no longer merge.

The single-row shard counting bug is being fixed there in NVIDIA/cosmos-framework#317, which writes a trailing newline per row. That PR does not include the rest of your change: the per-modality coverage check before evaluation, the set -u fix in the worker-pid wait loop, and test_run_paibench_c.sh. Those are still worth having. Please open a PR against NVIDIA/cosmos-framework with them (the file paths are the same under .agents/skills/), and reference #376 and cosmos-framework#317.

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.

PAIBench-C quick start generates no videos: pending-shard count misreads unterminated JSONL

2 participants