[refactor] Remove legacy inference codepath#1835
Merged
Merged
Conversation
Removes the legacy Ray-actor inference implementation, leaving only the HTTP-based path (formerly gated by _SKYRL_USE_NEW_INFERENCE=1, now the only path). The private feature flag is deleted entirely. - Delete skyrl/backends/skyrl_train/inference_engines/ (legacy client, remote/ray-wrapped engines, vLLM server/engine) and the legacy inference_servers/vllm_worker.py (WorkerWrap). - Relocate the still-shared pieces out of inference_engines/ before deleting it: the InferenceEngineInterface + I/O types move to inference_servers/base.py, and the shared helpers (build_engine_runtime_env, route_prompts_to_engines, get_sampling_params_for_backend, ...) move to a new lightweight inference_servers/engine_utils.py (kept import-light to avoid a cycle with server_group.py). All import sites repointed. - Weight sync: drop the legacy _send_chunks_legacy branches and the now-dead receiver side (Broadcast/CudaIpc WeightTransferReceiver, create_receiver, WeightTransferReceiver ABC, strategy_type/for_engine). The receive side is vLLM's native weight-transfer engine driven by NewInferenceWorkerWrap. - Collapse every _SKYRL_USE_NEW_INFERENCE branch to the new path across entrypoints, generators, workers, the Tinker backend, and train utils; remove the env var and its Ray runtime-env propagation. - Tests: delete legacy-only suites (http endpoint, transfer-strategies-e2e, pause/continue, weight_sync_moe, remote_weight_loader, new-vs-old); de-gate shared fixtures/tests; move test_route_prompts_to_engines into inference_servers/. - CI: remove the gpu_skyrl_train_old_inference workflow and its two scripts. - Examples/docs: remove the flash_rl example; migrate the remote_inference example and r2egym rollout scripts to the serve entrypoint / vLLM-native server; scrub legacy references across docs. Verified: ruff/black clean; CPU suites (weight_sync, transfer strategies, route-prompts, generators, trainer, Tinker sample-routing) all pass; GPU CI inference_servers/ (32 passed) and test_policy_local_engines_e2e.py (6 passed) pass on H100. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Follow up to removing the legacy inference codepath. Moves the last pieces that still assumed the old path over to the HTTP based inference client and server. - RemoteInferenceClient now inherits from InferenceEngineInterface. Trimmed the interface down to what the client actually implements and added a get_endpoint_url method so the generators can resolve the endpoint without depending on the concrete client type. - The mini swe agent, harbor and verifiers generators read the endpoint url at runtime instead of a fixed host and port. The mini swe agent sets the base url inside the Ray task that runs the trajectory. - Removed the http_endpoint_host and http_endpoint_port options along with every reference to them in scripts, tests and docs. - The tinker backend now puts the inference engines to sleep at initialization when training and inference are colocated, matching the old behaviour. - Added a standalone entrypoint to the vllm server actor module so it can be launched directly with python -m, serving the vllm openai server together with our custom endpoints, and pointed the thunder agent at it. - Removed route_prompts_to_engines and hash_with_sha256, which only the old path used. Also makes the gsm8k end to end test run name unique so that concurrent runs in the shared weights and biases project do not collide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
A round of cleanups on top of the new inference path, after merging vllm 0.23.0. - Weight sync: remove the unused WeightLoader; make BroadcastTransferStrategy.create_init_info take a required inference_world_size and drop the None branch; add a get_vllm_transfer_engine reference method to each strategy (NCCL and CUDA IPC); and drop the dead torch.distributed era fields from BroadcastInitInfo. - InferenceEngineInterface: add get_world_size, finish_session, render_chat_completion and a model_name property so the interface covers every client member the generators rely on. RemoteInferenceClient and OpenRouterInferenceClient already satisfy these. - Thunder agent: kill the whole process tree of each rollout server on cleanup so the vLLM engine core is not orphaned on the GPU. - Standalone server entrypoint test: tear down the server process tree so the test never leaks a GPU, and check a SkyRL specific endpoint. - Docs: update the weight sync layout and the inference architecture endpoint names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
- weight sync GPU test: drop the BroadcastInitInfo fields removed in the cleanup (group_name / backend / model_dtype_str) so the broadcast weight-sync test stops crashing. - mini-swe: append /v1 to OPENAI_BASE_URL so LiteLLM's openai provider hits the router's OpenAI routes instead of 404ing. - OpenRouterInferenceClient.generate now accepts the model arg from the interface; harbor generator type-hints InferenceEngineInterface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
…p_endpoint Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
SumanthRH
marked this pull request as ready for review
July 1, 2026 17:49
Contributor
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
hao-aaron
added a commit
to hao-aaron/SkyRL
that referenced
this pull request
Jul 1, 2026
Reconciles our vLLM 0.23.0 native weight-sync cleanup with main's NovaSky-AI#1835 "[refactor] Remove legacy inference codepath" (which removed the local Ray-actor engine path and the _SKYRL_USE_NEW_INFERENCE flag). Conflict resolutions: - inference_engines/ray_wrapped_inference_engine.py, remote_inference_engine.py: deleted on main. Accepted deletion — our changes are superseded: the vLLM version is now hard-pinned to 0.23.0 in pyproject.toml (our runtime assertion is moot), and token-in-token-out is now native in vllm_server_actor.py's /skyrl/v1/generate (returns resp.token_ids directly), so our return_token_ids HTTP change is no longer needed. - test_pause_and_continue_generation.py: deleted on main. Accepted deletion. - new_inference_worker_wrap.py: kept our near-empty hook + #44814 patch_numel_loaded (main had NOT done this cleanup; its version still carried the collective_rpc update_weights_ipc/nccl methods). - broadcast_strategy.py / cuda_ipc_strategy.py / remote_inference_client.py: auto-merged — main dropped the legacy _SKYRL_USE_NEW_INFERENCE dispatch and _send_chunks_legacy, our native-endpoint sender edits apply on top. Verified coherent (no leftover refs to removed symbols). - .claude/docs/weight_sync.md: dropped legacy WorkerWrap sections, kept our accurate native-endpoint lifecycle description. Follow-up: LayerwiseReloadWorkerMixin in layerwise_reload.py is now dead code (patch_numel_loaded/get_numel_loaded there are still used). Co-Authored-By: Claude Opus 4.8 (1M context) <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.
What does this PR do?
Summary
This PR removes the old Ray actor-based inference stack and standardizes SkyRL on the HTTP/vLLM-based inference path. The
_SKYRL_USE_NEW_INFERENCEflag is removed entirely, the legacyinference_engines/module is deleted, and the remaining training, generation, examples, and docs surfaces now target the HTTP/vLLM server path.The PR also migrates several examples and integrations that still assumed the old path, fixed/static inference ports, or the legacy custom vLLM server module.
Changes made
Core
Legacy inference stack removal
skyrl/backends/skyrl_train/inference_engines/package, including the old Ray-wrapped inference client, remote inference engine, HTTP endpoint client, vLLM engine wrapper, and old custom vLLM server.inference_servers/vllm_worker.pywrapper._SKYRL_USE_NEW_INFERENCEand collapsed all old/new inference branches onto the HTTP/vLLM path.inference_servers/base.py.inference_servers/engine_utils.py.Inference interface and remote client
InferenceEngineInterfacearound the methods actually used by the HTTP/vLLM path.RemoteInferenceClientimplement the interface directly so the interface and concrete client cannot drift.model_nameget_endpoint_url()get_world_size()render_chat_completion()finish_session()InferenceEngineInterfaceandget_endpoint_url()instead of reaching intoRemoteInferenceClient.proxy_urlwhen possible.served_model_nameis passed into vLLM and used correctly byRemoteInferenceClientand server setup.vLLM server entrypoint
skyrl.backends.skyrl_train.inference_servers.vllm_server_actorso it can also be launched as a standalone module:python -m skyrl.backends.skyrl_train.inference_servers.vllm_server_actorWeight sync cleanup
WeightLoader.NewInferenceWorkerWrap.SkyRL'sWeightTransferReceiverabstractions have been removed for now.NCCLWeightTransferEngineIPCWeightTransferEngineBroadcastTransferStrategy.create_init_infoby requiring the inference world size fromclient.get_world_size().Config and routing cleanup
generator.inference_engine.http_endpoint_hostgenerator.inference_engine.http_endpoint_portgenerator.inference_engine.enable_http_endpointgenerator.inference_engine.async_enginereferences across docs and examples. The config loader now tolerates/removes legacyasync_engine=trueoverrides but rejects unsupportedasync_engine=false.route_prompts_to_enginesandhash_with_sha256.Tinker updates
SkyRLTrainBackendso colocated inference engines are put to sleep after initialization, matching the colocated behavior with the python entrypoint and freeing memory before training work starts.tinker-cookbookcommit for reproducibility.Examples
MiniSWE Agent example
MiniSWEAgentGeneratorto the new inference interface.inference_engine_client.get_endpoint_url().OPENAI_BASE_URLis set at runtime inside the Ray task that runs MiniSWE-Agent, because LiteLLM reads that environment variable from the task process.OPENAI_BASE_URLassumption from.env.miniswe; the router port is dynamic in the new inference path.Thunder-Agent example
skyrl.backends.skyrl_train.inference_servers.vllm_server_actorvllm.entrypoints.openai.api_server, because SkyRL still needs custom endpoints for weight sync and inference-server control.Remote inference server examples
remote_inference_engineexample intoremote_inference_server.Harbor, Verifiers, RLM, and OpenRouter examples
get_endpoint_url()instead of removed fixed host/port config.InferenceEngineInterface.enable_http_endpoint/async_engineoverrides from Harbor, Verifiers, OpenEnv/OpenReward, RLM, and related integration scripts.General training scripts
generator.inference_engine.async_engine=trueand old HTTP endpoint overrides from the example training scripts.Removed obsolete examples
Docs
Architecture and configuration docs
_SKYRL_USE_NEW_INFERENCE, the old Ray actor inference stack, and deleted legacy classes.enable_http_endpoint, andasync_engineknobs.Example docs
Internal contributor docs
.claude/docsarchitecture, inference, contributing, and weight-sync notes to match the new inference and weight-sync structure.Tests and CI cleanup
tests/backends/skyrl_train/inference_engines/.RemoteInferenceClientserved_model_nameValidation
ruffandblackpass.