Repository navigation
feat: label sampled tokens with the policy step and log per-token policy age (vLLM 0.32 weight_versions) - #4141
Merged
Merged
Conversation
This was referenced Oct 18, 2026
faresobeid
marked this pull request as ready for review
October 19, 2026 00:07
# Conflicts: # src/prime_rl/orchestrator/clients.py
…abels # Conflicts: # uv.lock
faresobeid
added a commit
that referenced
this pull request
Oct 22, 2026
Ship per-token policy versions from the engines' weight_versions labels (#4141), falling back to the dispatch version for unlabelled tokens, instead of one dispatch version per sample.
## Summary - `node_policy_versions` counts sampled tokens with `np.count_nonzero(node.mask)` instead of `sum(node.mask)`. `MessageNode.mask` is a numpy bool array since PrimeIntellect-ai/verifiers#2961, which #4226 pulled into main. Python's `sum()` walks that array one token at a time and returns an `np.int64`. `np.count_nonzero` is vectorized, accepts lists too, and the `int()` cast keeps the metric values as plain ints. - `test_token_policy_ages` builds its masks as numpy arrays, which is the type main now produces. Targets #4141. Checks: `ruff check` and `ruff format --check` pass. I also ran `token_policy_ages` on the test case with list masks and with numpy masks. Both returned `(20, 8, 3)` as plain ints. I did not run pytest, because the import chain needs `torch` and the test machine (macOS) does not have it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small metric-calculation fix for numpy masks; behavior unchanged aside from correct int types and performance. > > **Overview** > **Fixes sampled-token counting** now that `MessageNode.mask` is a numpy bool array (from verifiers): `node_policy_versions` uses `int(np.count_nonzero(node.mask))` instead of `sum(node.mask)`, avoiding slow element-wise summation and keeping metrics as plain Python `int`s rather than `np.int64`. > > The unit test builds trace masks with `np.array(...)` to match production types. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5a96878. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
faresobeid
pushed a commit
that referenced
this pull request
Oct 24, 2026
## Summary - `node_policy_versions` counts sampled tokens with `np.count_nonzero(node.mask)` instead of `sum(node.mask)`. `MessageNode.mask` is a numpy bool array since PrimeIntellect-ai/verifiers#2961, which #4226 pulled into main. Python's `sum()` walks that array one token at a time and returns an `np.int64`. `np.count_nonzero` is vectorized, accepts lists too, and the `int()` cast keeps the metric values as plain ints. - `test_token_policy_ages` builds its masks as numpy arrays, which is the type main now produces. Targets #4141. Checks: `ruff check` and `ruff format --check` pass. I also ran `token_policy_ages` on the test case with list masks and with numpy masks. Both returned `(20, 8, 3)` as plain ints. I did not run pytest, because the import chain needs `torch` and the test machine (macOS) does not have it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small metric-calculation fix for numpy masks; behavior unchanged aside from correct int types and performance. > > **Overview** > **Fixes sampled-token counting** now that `MessageNode.mask` is a numpy bool array (from verifiers): `node_policy_versions` uses `int(np.count_nonzero(node.mask))` instead of `sum(node.mask)`, avoiding slow element-wise summation and keeping metrics as plain Python `int`s rather than `np.int64`. > > The unit test builds trace masks with `np.array(...)` to match production types. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5a96878. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
mikasenghaas
approved these changes
Oct 24, 2026
faresobeid
added a commit
that referenced
this pull request
Oct 24, 2026
…pisodes (#4143) `max_off_policy_steps` now bounds the age of each trained **token** instead of each episode. On main, the dispatcher cancels every train group whose dispatch version is older than `(step-1) - max_off_policy_steps`, including finished members, and the sink drops queued traces by the same rule. A group ages out as a whole even when most of its tokens came from recent weights (in-flight `keep`-mode updates). With this PR: - Each sampled token carries the policy step that sampled it (the `weight_versions` spans from #4141, on main). Unlabelled tokens (Dynamo only) take their episode's dispatch version, which is the old rule for them. - The sink masks tokens older than `(step-1) - max_off_policy_steps` out of `mask`, `rl_weights`, `ce_weights` and `ref_kl_weights`, at the queue sweep and at batch ship. Fresher tokens of the same sample keep training with the sample's advantage and their own inference logprobs. - A sample is dropped only when nothing is left to train; a trace only when all its samples are dropped (`off_policy/dropped`). - Rollouts are never cancelled for staleness. The dispatcher's stale cancel, the `stale` cancel reason and the sink's whole-group void are removed. On Dynamo, a stale group now runs to completion and is dropped at the sink. - New metric `off_policy/token/masked`: fraction of the batch's sampled tokens masked for staleness. - No loss changes: every loss in `trainer/rl/loss.py` is token-level and reads the loss mask. **Validation:** `pytest tests/unit/orchestrator tests/unit/test_configs.py tests/unit/inference tests/unit/eval -m "not gpu"` (512 passed), ruff. Not run end-to-end on GPU. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes off-policy enforcement in the orchestrator training path (masking vs cancellation), which can alter throughput, batch composition, and train–inference mismatch; loss code is unchanged but what reaches the trainer does. > > **Overview** > **`max_off_policy_steps` now limits staleness per sampled token**, not per rollout or group. The train sink masks tokens older than `(step-1) - max_off_policy_steps` out of `mask`, `rl_weights`, `ce_weights`, and `ref_kl_weights` (using vLLM `weight_versions`; unlabeled Dynamo tokens use the episode dispatch version). Fresher tokens in the same sample still train; traces drop only when every sample has nothing left to train. > > **Rollouts are no longer cancelled for staleness.** The dispatcher’s pre–weight-update stale group drops, the `stale` `CancelReason`, and whole-group voiding in the sink are removed; `on_version_pending` only blocks scheduling across the swap. Queue sweeps and batch shipment apply the token masks instead of episode-level drops. > > **Observability:** new `off_policy/token/masked` rate; docs/config describe token-level behavior; cancellation metrics treat pipeline voids as overload/shutdown rather than stale. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 24b8b51. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Mika Senghaas <mail@mikasenghaas.de> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Engines label each sampled token with the policy step that produced it, and the orchestrator logs per-token policy age.
/update_weightstakesweight_versionand calls vLLM'supdate_weight_versionafter loading./load_lora_adaptertakes aweight_versionquery parameter and does the same. The orchestrator sends the step on both (filesystem, NCCL and NIXL transports).weight_versionsspans on every/inference/v1/generatechoice; renderers/verifiers on main already pass them through to the trace.token_policy_agesturns the spans on a trace's assistant nodes into per-token ages against the batch's training version. The orchestrator logsoff_policy/token/{mean,max}next to the episode-leveloff_policy/*./update_weightsbefore the dispatcher starts, so the boot step is labelled too. Only Dynamo engines (weight updates via/collective_rpc) keep the"default"label; those tokens are not counted.Validation
pytest tests/unit/orchestrator tests/unit/test_configs.py tests/unit/inference -m "not gpu"(445 passed), ruff,uv lock --check./update_weightswithweight_version=5, resumed. It returned[{"version": "default", "start": 0, "end": 1390}, {"version": "5", "start": 1390, "end": 4000}]; the next request returned[{"version": "5", "start": 0, "end": 8}].Upstream: vllm-project/vllm#59582 (in 0.32). #4143 is stacked on this PR.
Note
Medium Risk
Extends the weight-update and LoRA reload admin APIs to require
weight_versionand invoke vLLM'supdate_weight_version; a mismatch or missing field would break sync, but behavior is additive for training.Overview
Wires vLLM 0.32
weight_versionsthrough the weight-sync path so each sampled token is tagged with the policy step that produced it, and the orchestrator can report finer-grained off-policy signal than episode-level staleness.After full weight loads (
/update_weights) and in-place LoRA reloads (/load_lora_adapter), engines callupdate_weight_versionwith the training step the orchestrator already passes on those admin calls (filesystem / NCCL / NIXL and LoRA filesystem receive). Generate responses then carry per-token version spans on traces.New helpers
node_policy_versionsandtoken_policy_agesturn those spans into per-token ages vs the batch’s training policy; at ship time the orchestrator logsoff_policy/token/meanandoff_policy/token/maxalongside existing episodeoff_policy/*metrics. Tokens still labelleddefault(e.g. Dynamo engines that skip this path) are excluded from the token metrics.Reviewed by Cursor Bugbot for commit 0025738. Bugbot is set up for automated code reviews on this repo. Configure here.