[Sync][misc] #1975 engine-agnostic riders (data.py debug-assert widen + parallel_check sweep) - #156
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the parallel configuration checks in the Qwen3 test suite and relaxes the assertion thresholds for rollout log probabilities and entropy. Feedback suggests further relaxing these assertion bounds (to -15 for log probabilities and 12 for entropy) to prevent fragile test failures in CI, as the current thresholds remain too tight for realistic generation tasks.
| assert abs(reduced_log_dict["rollout/log_probs"] - reduced_log_dict["rollout/ref_log_probs"]) < 1e-8 | ||
| if "rollout/log_probs" in reduced_log_dict: | ||
| assert -0.5 < reduced_log_dict["rollout/log_probs"] < 0 | ||
| assert -1 < reduced_log_dict["rollout/log_probs"] < 0 |
There was a problem hiding this comment.
The assertion threshold for rollout/log_probs is still very tight at -1 (which corresponds to an average token probability of ~36.8%). For many generation tasks, the average token probability can naturally be lower, which will cause flaky CI failures. Consider relaxing this bound significantly (e.g., to -15) to prevent fragile test failures while still catching extreme anomalies or NaNs.
| assert -1 < reduced_log_dict["rollout/log_probs"] < 0 | |
| assert -15 < reduced_log_dict["rollout/log_probs"] < 0 |
| assert -1 < reduced_log_dict["rollout/log_probs"] < 0 | ||
| if "rollout/entropy" in reduced_log_dict: | ||
| assert 0 < reduced_log_dict["rollout/entropy"] < 0.5 | ||
| assert 0 < reduced_log_dict["rollout/entropy"] < 1 |
There was a problem hiding this comment.
The assertion threshold for rollout/entropy is still very tight at 1. For a model with a large vocabulary like Qwen (V ≈ 150k, max entropy ≈ 11.9), an average entropy of 1 is extremely low. If the model's generations become slightly more diverse, the entropy can easily exceed 1, leading to flaky CI failures. Consider relaxing this upper bound significantly (e.g., to 12) to avoid fragile test failures.
| assert 0 < reduced_log_dict["rollout/entropy"] < 1 | |
| assert 0 < reduced_log_dict["rollout/entropy"] < 12 |
…den + parallel_check sweep) Mirror slime #1975 (the 2 engine-agnostic riders only; version/build_conda/v0.5.12.post1 patch-dirs = N/A for vime): - data.py: widen rollout sanity asserts (log_probs -0.5->-1, entropy 0.5->1) - test_qwen3_0.6B_parallel_check.py: simplify TP/PP/CP sweep [1,2,4,8]->[1,2,4] via parallel_sizes + product<=num_gpus guard. #1985/#1986 NOT included here: their target test files are introduced by mega-A/B/C/G (not yet in main) -> must sequence after those mega-PRs merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
f10e5d9 to
ad68337
Compare
2 participants
Summary
The two engine-agnostic riders split out of slime #1975 (the rest — version.txt/build_conda/dead sglang patch-dirs — is N/A for vime; see #107):
data.pydebug-assert widen: log_probs-0.5→-1, entropy0.5→1test_qwen3_0.6B_parallel_checksweep[1,2,4,8]→[1,2,4]via parallel_sizes + skip whentp*pp*cp > num_gpusMirror slime. (#1985/#1986 — the larger test-shortening — are sequenced AFTER mega-A/G merge, tracked in #107.)
🤖 Generated with Claude Code