Repository navigation
Conversation
The sync merge took main's server_control.py, whose abort_servers_until_idle() already waits until every engine reports idle, but kept 0z5a-branch's report-only drain check at the end of abort(). That block reads server_abort, verify_server_drain and _ABORT_DRAIN_VERIFY_DEADLINE_S, none of which exist after the merge, so the first abort() of every rollout raised NameError: name 'server_abort' is not defined. abort() is now the same as on main. tests/test_server_idle_contract.py only covered the removed probe API (verify_server_drain, query_engine_inflight, abort_inflight_requests) and failed at import; main's tests for abort_servers_until_idle cover the replacement. test_abort_deletes_inflight_without_pause_resume patched the removed verify_server_drain; without that patch it is the same as on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
main removed --keep-old-actor in the Slime v0.4.0 sync (vllm-project#441), and the sync merge dropped the argument, but two checks added on 0z5a-branch by the external-draft collector (vllm-project#454) still read it: - actor.py, the condition for reusing log-probs in the loss, raised AttributeError: 'Namespace' object has no attribute 'keep_old_actor' on the first train step whenever the terms before it hold, as in a plain GRPO run (one micro-batch group, policy loss, kl_coef 0); - arguments.py, the collect-only validation, raised the same error for every collect-only configuration that passed the earlier checks. Both terms are dropped. The option no longer exists, so they were always false; the log-prob reuse condition is now the same as on main. The collect-only test case that set keep_old_actor=True is removed with it, and the draft-feature-collection docs (en, zh) no longer ask users to disable the flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
…og-prob forward The sync merge took main's compute_log_prob(), which has no draft_feature_collector parameter, and dropped the lines in train_actor() that create the collector, but kept the call that passes it. Every run that needs a separate actor log-prob forward (can_reuse_log_probs_in_loss is false, e.g. kl_coef != 0, more than one micro-batch group, a critic or GSPO) raised NameError: name 'draft_feature_collector' is not defined, and --draft-feature-mode collect-only could not collect anything. Both pieces are restored as they were on 0z5a-branch (vllm-project#454), on top of main's code: train_actor() creates the collector in collect-only mode, and compute_log_prob() passes it to forward_only(), which still accepts it. This also uses the unwrap_model import again. tests/test_draft_feature_collector.py gains two CPU checks on the real methods (isolated with ast, as tests/test_reference_top_p_replay.py does): compute_log_prob() hands the collector to forward_only(), and train_actor() assigns every collector it passes. Both fail on the merge commit. The fake forward_only() in test_reference_top_p_replay.py accepts the new keyword. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
…-project#454 The sync merge kept the first half of 0z5a-branch's _inference_generate_tokens_and_logprobs() (the docstring and the token_ids check) but took main's second half, which zips token_ids with whatever logprobs.content holds. A response with fewer log-prob entries than tokens was silently truncated, and a missing, non-numeric or non-finite log-prob (NaN, inf, True, 10**1000) went straight into training. The try/except in generate() that adds the request id to these errors was dropped too, which left `import math` unused (ruff F401) and fails the pre-commit CI step. The parser tail and the wrapper are restored as they were in vllm-project#454, on top of main's generate(). vllm-project#454 also changed main's test_inference_generate_tokens_and_logprobs_aligns_partial_content to expect the length-mismatch error; the merge took main's version of that test, so it is restored to 0z5a-branch's. Fixes 11 tests in tests/test_vllm_rollout.py that fail on the merge commit: test_inference_logprob_rejects_invalid_numbers (6) and test_bad_metadata_does_not_mutate_training_fields (5). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
fdc372c removed tests/test_server_idle_contract.py together with the leftover drain check it tested, but the synchronized upstream CPU step in .buildkite/pipeline.yml still ran it. pytest exits non-zero on a missing file, so that step would fail as soon as pre-commit passes. main's pipeline does not list the file either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
Both tests come from one side of the sync merge and now run against the other side's code. Each fails on the merge commit. - test_colocate_offload_resolution (from vllm-project#454, 18 cases) checked the namespace it passed to vime_validate_args(). Since vllm-project#441 on main that function validates a deep copy and returns it, so the test now reads the returned args, as the other tests in the file do. - test_generate_requests_sampler_topk and test_generate_requests_complete_top_p_probabilities (from main) expected generate() to post without headers. vllm-project#454 tags every generate request with an x-request-id header (the id that the metadata errors restored in the previous commit report), so they now expect exactly that header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
main reorganised the docs into topic index pages and builds them with -W in CI. The sync merge took main's index pages, so two 0z5a-branch pages are no longer in any toctree, and Sphinx reports "document isn't included in any toctree" for each of them in en and zh. Under -W that fails the docs lab browser step. - advanced/draft-feature-collection.md (vllm-project#454) is listed in rollout/index.rst right after speculative-decoding.md, where it sat in 0z5a-branch's Advanced Features toctree. - The synced examples/ouro README (vllm-project#459) is listed in examples/workflows/index.rst next to the DSpark example. 0z5a-branch never listed it, but its pipeline did not build the docs with -W. `docs/build.sh {en,zh} -W --keep-going` now passes for both languages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
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.
Incremental review layer for vllm-project#506: fixes for the runtime errors and CI failures left by the vllm-project#483 sync merge (7 commits).
Base:
sync-0z5a-branch-to-main(vllm-project#483,0z5a-branchupdated tomain), so Files changed shows this layer only. The cumulative upstream PR is vllm-project#506.