Repository navigation
feat(cli): gym dev compare, the per-task parity report (M17) - #4257
Conversation
978f702 to
0508c85
Compare
0508c85 to
ee48566
Compare
yuhezhang-ai
left a comment
yuhezhang-ai
left a comment
There was a problem hiding this comment.
Reviewed M17 at ee485666 against M16 (#4256, 4e9672df). Please address attempt reconciliation, anonymous-failure matching/rendering, and run-specific verifier-log lookup before relying on this report as the migration gate. I reproduced the issues below with focused fixtures; the displayed-average suggestion is non-blocking.
These findings concern M17's handling of existing M16 records. Focused tests and lint passed, but the reproduced cases need coverage. Validation used saved-record fixtures; I did not repeat the production parity runs.
ee48566 to
6398074
Compare
fc10a8a to
5e2d867
Compare
5e2d867 to
49df761
Compare
49df761 to
02f40d5
Compare
02f40d5 to
004bf21
Compare
|
/ok to test 004bf21 |
|
🌿 Preview your docs: https://nvidia-preview-pr-4257-2cf15a400e50.docs.buildwithfern.com/nemo/gym |
github-actions
Bot
removed
the
sla:author-overdue
ea1b9f2 to
673187c
Compare
|
/ok to test 673187c |
|
/ok to test bd95952 |
|
/ok to test ff22bba |
|
/ok to test 7924b86 |
|
Status at the new head 7924b86. All five threads are answered inline and four are resolved: superseded failures are dropped before grouping, masked entries render with an absent side, verifier logs resolve against the losing row's own run, and masked placeholders are left out of the mean. The anonymous-failure thread had a second case, a side holding only anonymous sidecar failures, which now borrows the other side's key or falls back to _ng_task_id with every row listed as a masked unknown (tests/unit_tests/test_dev_compare.py covers both directions and both sides anonymous). The module is now the nemo_gym/rollout_compare package after the split. |
yuhezhang-ai
left a comment
yuhezhang-ai
left a comment
There was a problem hiding this comment.
Rechecked M17 at 7924b86f against its immediate parent, M16 (#4256, b7ede71b). All previous findings are fixed, including anonymous-only failure inputs without an explicit --key.
Validation: 389 comparison/CLI tests and 20 saved reproductions and targeted controls passed. The previously failing CLI fixture now succeeds in both text and JSON modes; focused lint and formatting checks also passed.
Approving the M17 changes. The failing Harbor partial-evaluation CI test also fails on #4256 and still needs resolution before merge.
|
/ok to test 76d8b5c |
|
/ok to test 14a7162 |
|
/ok to test 4f6a3ab |
|
/ok to test c3d5a2c |
ffrujeri
left a comment
ffrujeri
left a comment
There was a problem hiding this comment.
Approving c3d5a2c3. gym dev compare is self-contained in nemo_gym/rollout_compare. The join handles the cases that matter for parity reports: a namespace prefix only one side carries, task IDs that recur across tasksets (reported as ambiguous rather than guessed), anonymous sidecar failures kept out of the join, and repeated rows compared on the mean reward with a note. Compare and CLI suites pass locally (356 passed), and CI is green apart from the non-blocking conformance probes.
|
/ok to test a4df217 |
|
/ok to test 5e27c4f |
`gym dev compare OLD NEW [--logs-root DIR] [--tail N] [--json PATH]` joins two rollout files by
task and reports identical, flipped (old-win / new-win), flipped (agent error), masked and missing
tasks, with the verifier output tail for every flip read under each row's `verifier_logs_dir`.
It is the gate before a legacy server is retired in favour of its Harbor-path replacement.
Review hardening carried over: `--json` to an unwritable path is one error line and exit 1; the
root `gym --json` toggle prints the summary on stdout; paths and quoted options reach the command
verbatim (a `${` is escaped through the Hydra merge); a side is keyed by taskset only when it
spans more than one and a plain side joins on task id; a scored row that carries a
`failure_kind` with `mask_sample` false counts as measured (the Harbor environment server scores
a verifier timeout or a missing reward file that way); and a sidecar failure that names no task
stays out of the vote and is listed as masked.
Squashed from the previous branch.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Adil Asif <adasif@nvidia.com>
|
/ok to test 2cf15a4 |
ffrujeri
left a comment
ffrujeri
left a comment
There was a problem hiding this comment.
Re-approving 2cf15a40 after the restack. git range-diff shows the patch is identical to the c3d5a2c3 I approved.
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/39688243980 |
What does this PR do?
RFC M17, the temporary parity command.
gym dev compare <old rollouts.jsonl> <new rollouts.jsonl>joins rows by task, counts identical, flipped (old-win and new-win), masked and missing, prints the losing side's verifier output for every flip, and writes a JSON summary with--json PATHor on stdout withgym --json dev compare. It replaces the audit scripts used for the Terminal Bench 2.1 and 4 parity runs and is the gate before an old server is deleted (M39). It lives undergym devas a migration aid.nemo_gym/rollout_compare.py: loading (with the<name>_failures.jsonlsidecar folded in as masked rows), task identity detection (_ng_task_id,task_name, legacy prefixes,--key), per-side taskset keying with ambiguous ids reported rather than guessed, classification, repeats compared on the mean, verifier output fromverifier_logs_dir(resolved against each file's directory or--logs-root) or the embeddedtest_output, a separate bucket for flips where a side reports an agent error ("rerun before counting").nemo_gym/cli/dev.py,nemo_gym/cli/main.py: command wiring; positional paths and quoted options reach the config verbatim (non-ASCII, spaces, backslashes,${).--jsonpath exits 1 before anything is printed; an unreadable verifier log is reported per flip and the report continues; NaN and infinite rewards are masked.fern/versions/latest/pages/reference/cli-commands.mdx.Testing
76 tests in
tests/unit_tests/test_dev_compare.py, 300 across the CLI suite. On the Terminal Bench 2.1 parity v3 files it reproduces the hand audit exactly: 72 identical, 14 flipped (8 old-win, 6 new-win), 1 flipped with an agent error, 2 masked, 0 missing, and every flip's verifier tail matches the audit byte for byte. Two review rounds with refutation and reproduction, 22 findings, all fixed.🤖 Generated with Claude Code