Skip to content

[CI] add reward-fn cpu tests + cpu-on-main trigger (port slime #1939+#1940) - #129

Closed
aoshen02 wants to merge 1 commit into
mainfrom
sync/slime-ci-1939-1940
Closed

[CI] add reward-fn cpu tests + cpu-on-main trigger (port slime #1939+#1940)#129
aoshen02 wants to merge 1 commit into
mainfrom
sync/slime-ci-1939-1940

Conversation

@aoshen02

@aoshen02 aoshen02 commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Ports THUDM/slime#1939 (add more cpu ci) + #1940 (run cpu test on main).

What

  • tests/test_rm_{deepscaler,f1,gpqa,math,math_dapo}.py — pure-cpu unit tests for vime/rollout/rm_hub reward fns. 107 tests, validated locally (all pass).
  • vime/utils/misc.py — lazy import ray so cpu-only paths don't require ray.
  • pr-test.yml.j2 — register the 5 rm tests in the cpu (num_gpus:0) matrix; enable push-to-main trigger gated to cpu jobs only (GPU/self-hosted jobs stay PR/label-gated, so push never burns the fleet).

Port notes (sglang→vllm / vime divergence)

Validation

  • Local: pytest tests/test_rm_*.py → 107 passed.
  • ruff clean. CI (cpu job) will run the rm tests on this PR.

Part of the #107 slime→vime sync. 🤖 Generated with Claude Code

…#1940)

Port of THUDM/slime#1939 (add more cpu ci) + #1940 (run cpu test on main):
- tests/test_rm_{deepscaler,f1,gpqa,math,math_dapo}.py: pure-cpu unit tests
  for vime/rollout/rm_hub reward fns (107 tests, validated locally).
- vime/utils/misc.py: lazy 'import ray' so cpu-only paths (rm_hub scoring,
  plugin contracts) don't require ray.
- pr-test.yml.j2: register the 5 rm tests in the cpu (num_gpus:0) matrix;
  enable push-to-main trigger gated to cpu jobs only (GPU/self-hosted jobs
  stay PR/label-gated so push never burns the fleet).

slime #1939 also added tests/test_sample.py, which references Sample.rollout_id
(introduced by the #1926-chain / vime PR #119) — that test is stacked on #119
instead of here.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds comprehensive CPU unit tests for several reward model scoring modules, including deepscaler, f1, gpqa, math_utils, and math_dapo_utils. It also updates vime/utils/misc.py to lazily import ray inside get_current_node_ip(), allowing CPU-only paths to run without requiring the ray library. The review feedback correctly identifies two issues in the new tests where integer values were used instead of floats, which prevents the float coercion and parsing logic from being properly exercised in test_rm_deepscaler.py and test_rm_math_dapo.py.

"""float labels: stringified to e.g. "42.0". The current grader path
(mathd or sympy) handles "42.0" vs "42" via normalization — pinning
the wiring, not the equality logic."""
assert get_deepscaler_rule_based_reward(r"</think>\boxed{42}", 42) == 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The test is intended to verify float labels, but it passes the integer 42 instead of the float 42.0. This makes it identical to test_label_as_int_is_coerced_to_string and fails to actually test float coercion.

Suggested change
assert get_deepscaler_rule_based_reward(r"</think>\boxed{42}", 42) == 1
assert get_deepscaler_rule_based_reward(r"</think>\boxed{42}", 42.0) == 1

"""Minerva path expects gt to coerce via ``int(float(gt))`` (line 210),
so floats / int-strings collapse to canonical int strings before
comparison."""
correct, pred = is_correct_minerva("Long solution. Answer: 42", "42")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The test docstring mentions that floats collapse to canonical int strings, but the test currently passes "42" as the ground truth, which does not exercise the float parsing path. Passing "42.0" instead would properly validate this behavior.

Suggested change
correct, pred = is_correct_minerva("Long solution. Answer: 42", "42")
correct, pred = is_correct_minerva("Long solution. Answer: 42", "42.0")

@aoshen02

aoshen02 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

Closing for now (slime→vime sync being restructured). Branch is preserved — can reopen anytime.

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