[trainer] feat: generic remote backend abstraction for RL Training - #6422
Merged
wuxibin89 merged 38 commits intoJul 16, 2026
Merged
Conversation
Mirrors snowflake-eng/arctic-verl#22, rebased onto upstream verl v0.7.1 (bec9ef7) so the verl team can review the generic remote-backend abstraction in isolation. Adds: * `verl/remote_backend/{base,trainer,worker,worker_utils}.py` — a 9- method `RemoteBackend` ABC, `RemoteBackendRegistry` (lazy-imports adapters via a `name -> dotted-path` map), `RemoteBackendTrainer` (RayPPOTrainer subclass that drives a backend via `from_config` and threads a `reconnect_handle()` into every forwarder), and a CPU-only forwarder worker that delegates compute_log_prob / update_actor / update_weights / save_checkpoint to the backend and rebuilds NJTs + FlopsCounter MFU on the way back. * `verl/trainer/ppo/arctic_rl_client.py` — the Arctic adapter, registered as `"arctic"` via `@RemoteBackendRegistry.register(...)`, and `verl/workers/rollout/arctic_rollout/` — the Arctic vLLM rollout replica that re-attaches via `from_config(handle=...)`. * `trainer.remote_backend` config field in `ppo_trainer.yaml` and the one-line conditional in `main_ppo.py` that selects `RemoteBackendTrainer` + the generic forwarder when the field is set. The standard in-process verl path is untouched when it isn't. * One canonical example (`examples/arctic_rl/run_bird_grpo_arl_zorro_yes.sh`) plus baseline scripts and a reward helper for parity with the Arctic public PR. Reviewer notes: * Backend payload schemas, wire formats, and loss-function plumbing stay inside each backend; the ABC says nothing about them. * `RemoteBackend.requires_single_forwarder() -> True` for Arctic; the trainer asserts `n_gpus_per_node * nnodes == 1` and a single rollout replica because the backend owns its own training / sampling parallelism downstream. * Adapters live in `RemoteBackendRegistry.MODULES`; adding a new backend (e.g. `"tinker"`) is one map entry + one decorated class. This branch is the parallel of snowflake-eng/arctic-verl#22 rebased onto upstream v0.7.1. Co-authored-by: Cursor <cursoragent@cursor.com>
…traction-public Generic RemoteBackend abstraction + Arctic adapter (parallel to PR #1)
sfc-gh-truwase
requested review from
ArronHZG,
PeterSH6,
eric-haibin-lin,
tongyx361,
vermouth1992,
wuxibin89 and
zw0610
as code owners
May 20, 2026 21:55
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a generic RemoteBackend abstraction to verl, enabling the framework to drive out-of-process RL backends for training, rollout, and checkpointing. Key additions include the RemoteBackend base class, a specialized RemoteBackendTrainer, and a backend-agnostic forwarder worker, with an initial implementation provided for the Arctic backend. Review feedback identifies several critical issues: the profiler's async wrapper lacks necessary instrumentation, a hardcoded model path exists in the Arctic engine, and synchronous blocking calls are made within async worker methods, violating Ray best practices. Additionally, an indentation error was found in the DeepSpeed configuration logic, and the manual override of CUDA_VISIBLE_DEVICES was flagged as a potential conflict with Ray's resource management.
Comment on lines
+167
to
+172
| @functools.wraps(func) | ||
| async def async_wrapper(self_instance, *args, **kwargs_inner): | ||
| # Nested profiler annotate paths assume sync callables; async methods run uninstrumented. | ||
| return await func(self_instance, *args, **kwargs_inner) | ||
|
|
||
| return async_wrapper |
Contributor
There was a problem hiding this comment.
The async_wrapper implementation does not use the profiler's annotate context manager, which means any async function decorated with @DistProfiler.annotate will not be instrumented. This is particularly problematic as the new RemoteBackendActorRolloutRefWorker uses async methods for its core operations (e.g., compute_log_prob, update_actor). The wrapper should be updated to include the profiling instrumentation.
Suggested change
| @functools.wraps(func) | |
| async def async_wrapper(self_instance, *args, **kwargs_inner): | |
| # Nested profiler annotate paths assume sync callables; async methods run uninstrumented. | |
| return await func(self_instance, *args, **kwargs_inner) | |
| return async_wrapper | |
| @functools.wraps(func) | |
| async def async_wrapper(self_instance, *args, **kwargs_inner): | |
| profiler = getattr(self_instance, "profiler", None) | |
| if profiler is None: | |
| return await func(self_instance, *args, **kwargs_inner) | |
| with profiler.annotate(name=name, color=color, role=role, **kwargs_outer): | |
| return await func(self_instance, *args, **kwargs_inner) | |
Collaborator
|
Could you rename this pull request and to reflect the general-purpose of the code change? |
sfc-gh-truwase
changed the title
…kend.arctic` Addresses @zw0610's review on verl-project#6422 asking that core paths use generic names so other backends can dock without colliding with `arctic_rl`. Changes: * `verl/trainer/config/ppo_trainer.yaml`: top-level `arctic_rl:` block becomes `remote_backend.arctic:` (nested under a generic `remote_backend:` parent). Comment explains that new backends drop in as sibling blocks (`remote_backend.<name>:`) with no other changes required. * `verl/trainer/ppo/arctic_rl_client.py`: adapter now caches `self._backend_config = config.remote_backend[self._BACKEND_CONFIG_KEY]` in `__init__` and reads all backend-specific fields through it. `_BACKEND_CONFIG_KEY = "arctic"` is the single source of truth a new backend would override. * `examples/arctic_rl/run_gsm8k_grpo_arl_zorro_yes.sh`: 9 hydra overrides switched from `arctic_rl.*` to `remote_backend.arctic.*`. Adapter-side filenames/classes (`arctic_rl_client.py`, `ArcticRLClientWrapper`, `verl/workers/rollout/arctic_rollout/`) and the `"arctic"` rollout-replica registry entry are intentionally left alone since they're adapter-scoped and follow the same shape as the existing `vllm`/`sglang`/`trtllm` entries. Smoke: 6-step GSM8K GRPO on H200, MFU 148-174, matches pre-rename baseline. Co-authored-by: Cursor <cursoragent@cursor.com>
* `ppo_trainer.yaml`: drop `tiled_logits_compute` (advanced OOM-only
knob); adapter now reads it via
`self._backend_config.get("tiled_logits_compute", True)`.
* `arctic_rl_client.py`: flip in-code default for `colocate` from
`False` to `True` per Tunji's comment.
* Cherry-pick of arctic-verl@662e421b — adapted for the version of
`arctic_training` shipping in this PR (its client RPCs are still
sync; only `sync_weights` is async). Plumbs `async`/`await` through
the full stack so the forwarder's Ray event loop stays responsive,
also addressing the gemini-bot blocking-call comment on
verl-project#6422:
- `RemoteBackend.compute_log_prob`, `update_actor`,
`save_checkpoint` declared `async` on the ABC.
- `ArcticRLClientWrapper` implements them `async` and dispatches
the underlying sync `self._client.fwd_no_grad` /
`fwd_bwd` / `step` / `save_checkpoint` via
`asyncio.to_thread`, so the worker thread blocks instead of the
asyncio loop.
- `RemoteBackendActorRolloutRefWorker._run_log_prob` /
`_run_update_actor` made async and `await` the backend.
`save_checkpoint` / `destroy` likewise await.
- `RemoteBackendTrainer.destroy` (sync shutdown path) bridges
with `asyncio.run` to await the now-async `backend.destroy`.
* `arctic_rollout.py`: drop hardcoded `Qwen/Qwen3-0.6B` tokenizer
load. `ArcticLLMEngine.__init__` takes a tokenizer, and
`ArcticLLMServer` passes `self.model_config.tokenizer` (already
loaded by `HFModelConfig.__post_init__`), avoiding both a hardcoded
model name and a redundant download.
* `run_gsm8k_grpo_arl_zorro_yes.sh`: drop the now-redundant
`remote_backend.arctic.tiled_logits_compute=True` override (the
in-code default already gives `True`).
Smoke: 6-step GSM8K GRPO on H200, MFU 149-170 (matches pre-change
baseline 148-174); no regressions.
Co-authored-by: Cursor <cursoragent@cursor.com>
Closing `}` of the returned dict was indented at column 4 instead of column 8 — visually breaks the dict nesting, even though Python accepts it. Trivial cosmetic fix flagged by gemini-bot on verl-project#6422. Smoke: 6-step GSM8K GRPO on H200, MFU 146-175, matches baseline. Co-authored-by: Cursor <cursoragent@cursor.com>
`arctic_training.arctic_rl.client.ArcticRLRayClient.save_checkpoint` is `async` on the current `tunji/verl_integration` HEAD, so the `asyncio.to_thread` wrapper is unnecessary. `fwd_no_grad` / `fwd_bwd` / `step` are still sync upstream and continue to use `to_thread` (per inline-reply on PR #3 explaining the test). Smoke: 6-step GSM8K GRPO on H200, MFU 199-224 (post-arctic_training bump), no regressions. Co-authored-by: Cursor <cursoragent@cursor.com>
[refactor] Rename arctic_rl config namespace to generic remote_backend.arctic
wuxibin89
changed the title
Collaborator
|
@sfc-gh-truwase Could you please provide a RFC design for this? |
zw0610
reviewed
May 22, 2026
Now that ArcticTraining-dss commit af1ab8d (`Async+bf16 opt`) makes `fwd_no_grad`, `fwd_bwd`, and `step` async on `ArcticRLRayClient`, `asyncio.to_thread` is no longer needed for these three. Switching to bare `await self._client.<op>(...)` per Tunji's retry-await comments on PR #3 (lines 458, 459) — symmetry with the already-await'd `save_checkpoint` and `sync_weights`. Also drops the now-unused `import asyncio` from `arctic_rl_client.py`. `RemoteBackendTrainer.destroy` still uses `asyncio.run` to bridge the sync shutdown path to the async `backend.destroy`. Smoke: 6-step GSM8K GRPO on H200, MFU 186-218; matches the `asyncio.to_thread` baseline (199-224), no perf regression from dropping the thread hop. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses @zw0610's structural review on verl-project#6422: - #2: verl/trainer/ppo/arctic_rl_client.py -> verl/workers/remote_client/arctic_rl_client.py - #3: verl/workers/rollout/arctic_rollout/ -> verl/workers/rollout/remote_rollout/arctic_rollout/ - #4: TODO(@zw0610) on RolloutReplicaRegistry registrations in verl/workers/rollout/replica.py:360 about lazy-init each option to avoid pulling in vLLM + SGLang + TRT-LLM transitive deps eagerly. Pure code-motion + a TODO; no behavior change. Imports inside the moved files and in workers/rollout/replica.py updated. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses the remaining @zw0610 structural comments on verl-project#6422: - #1 (split remote_backend out of ppo_trainer.yaml): * New verl/trainer/config/remote_backend/arctic.yaml with the arctic block content. * In verl/trainer/config/ppo_trainer.yaml, replace the inline remote_backend: {} block with a Hydra optional defaults entry (`- optional remote_backend@remote_backend: null`). Users opt in with `remote_backend=arctic` (standard Hydra option group syntax). * Example script (run_gsm8k_grpo_arl_zorro_yes.sh) updated accordingly. - #5 (trim RemoteBackend ABC to lifecycle only): * Drop `compute_log_prob`, `update_actor`, `generate` from verl/remote_backend/base.py. ABC now enforces only: `from_config`, `reconnect_handle`, `destroy`, `update_weights`, `save_checkpoint`, `requires_single_forwarder`. * Concrete compute/update/generate methods stay on the Arctic adapter as plain methods (called from the Arctic-specific worker, not via the ABC contract). - #6 (empty MODULES, explicit registration): * Drop the lazy `RemoteBackendRegistry.MODULES` table from base.py so no transitive deps (vLLM, arctic-training, tinker, ...) get pulled in eagerly. * main_ppo.py now imports the adapter module explicitly (`from verl.workers.remote_client import arctic_rl_client`) when `trainer.remote_backend=arctic`; the import side-effect registers the class. - #7 (per-backend worker, not a generic parallel class): * New verl/remote_backend/workers/arctic_rl/ package. * Rename `RemoteBackendActorRolloutRefWorker` to `ArcticRLActorRolloutRefWorker` (the worker.py move happened in the previous commit). * main_ppo.py selects the per-backend worker class explicitly (currently arctic; extends backend-by-backend, with a clear error message on unknown backend names). * NOTE: this PR does NOT yet make the worker inherit from `verl.workers.engine_workers.ActorRolloutRefWorker` (Wang's full #7 prescription). Doing so requires first decoupling `ActorRolloutRefWorker.__init__` from megatron-specific config fields (e.g. `config.actor.megatron.router_replay`) that the Arctic config tree doesn't carry; left as a follow-up RFC item. Verified end-to-end via 6-step GSM8K GRPO smoke (Qwen3-0.6B, single-GPU, zorro=True); convergence and MFU unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
Ray child procs don't inherit the driver's import side-effects, so the
`@RemoteBackendRegistry.register("arctic")` decorator never ran in the
WorkerDict actor and `RemoteBackendRegistry.get("arctic")` raised
`KeyError`. Fix by eager-importing the adapter at the top of the
per-backend worker module — every process that loads this worker now
also loads (and registers) its adapter. The driver still imports the
adapter explicitly in `verl.trainer.main_ppo`; this is the matching
import in the worker process.
Caught by the 6-step GSM8K GRPO smoke after the registry/MODULES trim.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@wuxibin89 RFC draft coming separately; in the meantime, Snowflake-AI-Research/verl#4 implements @zw0610's structural restructure (stacked on #3), and is the cleanest snapshot of the proposed design today:
I'll send the full RFC doc (motivation, alternatives we considered, scaling/sharding story, follow-up items like full |
…ctic.yaml (Tunji PR#4) Addresses 2 outstanding @sfc-gh-truwase comments on PR #4: 1. `verl/workers/remote_client/arctic_rl_client.py` -> `arctic_rl.py` The new directory `remote_client/` already names the role, so the `_rl_client` suffix is redundant. Updated all 8 callsites (main_ppo.py, arctic_rollout.py, base.py, worker.py, __init__.py files, and adapter self-references in docstrings). 2. `trainer/config/remote_backend/arctic.yaml`: dropped the top-level `arctic:` namespace — the file name (`arctic.yaml`) is already the backend name, so nesting was redundant. Adapter now reads `config.remote_backend` directly (removed `_BACKEND_CONFIG_KEY` indirection). Updated the example shell script Hydra overrides (`remote_backend.arctic.X=Y` -> `remote_backend.X=Y`). Verified end-to-end: `verl.remote_backend`, the renamed adapter, the per-backend worker, and the rollout chain all import cleanly under the arctic_rl venv. Co-authored-by: Cursor <cursoragent@cursor.com>
Ported from recipe/rl-correctness 8e7e747f (Olatunji Ruwase). Drop the per-call `shift` plumbing through no_padding_2_padding and move the zorro response-alignment correction into the forwarder worker. - verl/workers/utils/padding.py: remove the `shift` arg; restore the single legacy "predict-next" slice (shift=-1) for all callers. - verl/trainer/ppo/ray_trainer.py: remove `_model_output_shift()` and its call sites (same path as upstream; pure revert). - newshape: upstream applied the response-aligned->predict-next shift in verl/workers/arctic_workers.py::compute_any_log_prob (which doesn't exist on this PR). The equivalent njt construction lives in the Arctic forwarder worker, so the shift helper moves to verl/remote_backend/worker_utils.py and is applied in ArcticRLActorRolloutRefWorker._run_log_prob when remote_backend.zorro_train.enable is set. Co-authored-by: Cursor <cursoragent@cursor.com>
verl abstract remote backend
…ss-deltas-on-share-v0.7.1 [arctic_rl] backport rl-correctness payload-encoding deltas onto arctic_rl_share_v0.7.1
Collaborator
|
@sfc-gh-truwase Hi, could you port this PR to our new V1 trainer |
…ency-fix Arctic rollout: override Ray's default max_concurrency (1000 -> 4096)
PPO mini batch training
23 tasks
…n-only; strip Arctic from verl core Follow-up to the RemoteBackend abstraction (verl-project#6422). Turns the existing Arctic-aware verl core into a truly generic + plugin-only shape so adapters can live entirely out-of-tree, loaded via the existing VERL_USE_EXTERNAL_MODULES hook. Zero behavioural change for verl's built-in rollout backends (vllm / sglang / trtllm) and for non-remote PPO paths; the trainer registers the plugin adapter on first `fit()`-time lookup exactly the way it registered the (previously hardcoded) Arctic worker. ## Why Today, `verl/trainer/main_ppo.py` hardcodes: if backend_name == "arctic": from verl.remote_backend.workers.arctic_rl import ( ArcticRLActorRolloutRefWorker, ) from verl.workers.remote_client import arctic_rl # noqa: F401 actor_rollout_cls = ArcticRLActorRolloutRefWorker else: raise ValueError(f"Unknown trainer.remote_backend={backend_name!r}. Known: 'arctic'. ...") and `verl/workers/rollout/replica.py` eagerly does: def _load_arctic(): from verl.workers.rollout.remote_rollout.arctic_rollout.arctic_rollout import ArcticReplica return ArcticReplica RolloutReplicaRegistry.register("arctic", _load_arctic) Both branches pull Arctic-specific modules (and their transitive deps: DeepSpeed, arctic_training, arctic_inference, vLLM) into every verl process the moment a user selects `trainer.remote_backend=arctic`, even though verl core has no need for the code and out-of-tree adapters can't slot in without patching main_ppo. ## What lands 1. `RemoteBackendRegistry` grows two symmetrical worker-side classmethods (`base.py`, ~85 lines): * `register_worker(name, loader)` -- plugin calls this to register a lazy loader (a zero-arg callable) for the `ActorRollout(Ref)` forwarder worker class. * `get_worker(name)` -- `main_ppo` calls this to resolve the worker class, running the loader once and caching the result. 2. `main_ppo.py` drops the hardcoded `if backend_name == "arctic"` branch and instead does: worker_cls = RemoteBackendRegistry.get_worker(backend_name) if worker_cls is None: raise ValueError(...) # names the plugin contract in the error actor_rollout_cls = worker_cls Error message points the user at `RemoteBackendRegistry.register_worker(...)` and `VERL_USE_EXTERNAL_MODULES` so the wiring failure mode is self-explanatory. 3. `workers/rollout/replica.py` drops `_load_arctic` and its eager `RolloutReplicaRegistry.register("arctic", _load_arctic)`; comment in place directs readers to the plugin path. 4. `worker_utils.py` docstring updated to note that per-backend forwarder workers now ship in adapter packages, not verl core. 5. Deletes 10 Arctic-specific files from verl core (all functionality moves to the out-of-tree plugin under `arctic_platform/integrations/verl/`): ``` verl/remote_backend/workers/__init__.py verl/remote_backend/workers/arctic_rl/__init__.py verl/remote_backend/workers/arctic_rl/worker.py verl/workers/remote_client/__init__.py verl/workers/remote_client/arctic_rl.py verl/workers/rollout/remote_rollout/__init__.py verl/workers/rollout/remote_rollout/arctic_rollout/__init__.py verl/workers/rollout/remote_rollout/arctic_rollout/arctic_rollout.py verl/trainer/config/remote_backend/arctic.yaml examples/arctic_rl/run_gsm8k_grpo_arl_zorro_yes.sh ``` Net: +144 / -1682 across 15 files. ## Companion Arctic-Platform PR Paired plugin package landed as Arctic-Platform commit `89bd4f7` (feat/integrations-verl) -- `arctic_platform/integrations/verl/` subpackage with a `register.py` that: * decorator-registers `ArcticRLClientWrapper` (via `@RemoteBackendRegistry.register("arctic")` on the class), * calls `RemoteBackendRegistry.register_worker("arctic", ...)` with a lazy loader for its `ActorRollout(Ref)` forwarder, * calls `RolloutReplicaRegistry.register("arctic", ...)` with a lazy loader for its `RolloutReplica`. Everything except the backend-class decorator is lazy; a plain `import arctic_platform.integrations.verl.register` stays cheap and takes on no DeepSpeed / vLLM / arctic_inference cost unless the selected backend at `fit()` is `"arctic"`. ## Smoke test Full 4-step GSM8K GRPO run (Qwen3-1.7B, single H200) against a verl tree at this commit + Arctic-Platform's plugin package loaded via `VERL_USE_EXTERNAL_MODULES=arctic_platform.integrations.verl.register` + `hydra.searchpath=[file://.../integrations/verl/config]`: | step | MFU (actor) | throughput (tok/s) | step time (s) | |---:|---:|---:|---:| | 1 | 0.261 | 5164 | 48.4 | | 2 | 0.256 | 5860 | 42.4 | | 3 | 0.260 | 6283 | 39.0 | | 4 | 0.264 | 6478 | 38.4 | `val-core/openai/gsm8k/acc/mean@1=0.00152` at step 4 (expected: near-zero for a 4-step GRPO run from a stock baseline; the goal is exercising the eval path). Weight sync runs over CUDA-IPC and the receiver logs `[weight-sync names validated] context=cuda_ipc sender=310 expected=310` on every step. No tracebacks; clean vLLM engine shutdown. Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced Jul 10, 2026
sfc-gh-kganesan
added a commit
to Snowflake-AI-Research/Arctic-Platform
that referenced
this pull request
Jul 10, 2026
…form/integrations/verl/ Ships the ArcticRL <-> verl adapter as an opt-in subpackage under `arctic_platform/integrations/`, plugged into verl at runtime via the `VERL_USE_EXTERNAL_MODULES` hook. Closes #35. ## Why verl core keeps its generic `RemoteBackend` ABC + registry; all Arctic-specific runtime code (client wrapper, per-backend worker, `RolloutReplica`, Hydra config, verl-shaped GRPO loss) now lives in one place inside `arctic_platform` and is discovered by verl at bootstrap. No verl source-tree patches, no forked launcher, no eager DeepSpeed / vLLM / arctic_inference imports for jobs that don't select the arctic backend. ## What lands New subpackage `arctic_platform.integrations.verl/`: | file | role | |---|---| | `register.py` | Loaded by `VERL_USE_EXTERNAL_MODULES`; eagerly imports `adapter` so `@RemoteBackendRegistry.register("arctic")` runs, then registers lazy loaders for the actor-rollout worker (`RemoteBackendRegistry.register_worker`) and the rollout replica (`RolloutReplicaRegistry.register`). | | `adapter.py` | `ArcticRLClientWrapper` -- verl's `RemoteBackend` ABC on top of `arctic_platform.rl`. | | `worker.py` | `ArcticRLActorRolloutRefWorker` -- CPU-only forwarder verl instantiates for the `ActorRollout(Ref)` role. | | `rollout.py` | `ArcticReplica` / `ArcticLLMServer` -- hosts Arctic's vLLM engine as a verl `RolloutReplica`. | | `grpo_loss.py` | Server-side verl-shaped GRPO loss (registered as `"verl_grpo"` on `arctic_platform.rl.processors.LOSS_FNS`). | | `config/remote_backend/arctic.yaml` | Per-backend Hydra config block; loaded via `remote_backend=arctic` with `hydra.searchpath` pointing at this dir. | | `examples/run_{gsm8k,bird}_grpo_arl.sh` | Reference single-GPU GRPO launchers matching Golden Runs 1 & 2. | Backward compatibility: `arctic_platform/rl/processors/verl_grpo.py` is now a one-line wildcard re-export of the new module, so the old import path (and its `LOSS_FNS["verl_grpo"]` side effect) still works. `pyproject.toml` gets a new `[verl]` extra and includes the plugin's yaml / example / README assets in the wheel. ## Recipe alignment `recipes/rl/verl/simple/`: - `run_qwen3_1.7b_gsm8k_grpo_arl.sh` now exports `VERL_USE_EXTERNAL_MODULES=arctic_platform.integrations.verl.register` and passes `hydra.searchpath=[file://.../integrations/verl/config]` to `verl.trainer.main_ppo`. No other launcher changes. - README clarifies that the Arctic backend ships as a plugin and that verl core carries no Arctic-specific files. ## Tests `tests/integrations/verl/` -- 17 tests, all green, all import-safe on machines without vLLM / DeepSpeed / arctic_inference (`conftest.py` stubs `verl.*`, `arctic_platform.rl.*`, and heavy transitive deps before importing the plugin): - `test_register.py`: decorator-based backend registration side effect, lazy worker + replica registration, loader-target verification via `inspect.getsource`. - `test_adapter.py`: `ArcticRLClientWrapper` API surface + `destroy()` idempotency. - `test_payload.py`: `_prepare_padded_arctic_batch_dict` shape / dtype invariants over nested-tensor inputs. - `test_backward_compat.py`: the shim re-exports the same function object and preserves the `LOSS_FNS["verl_grpo"]` registration. ## Companion verl-core change Paired PR: verl-project/verl#6422 (generic `RemoteBackend` ABC + registry + `register_worker` / `get_worker` on the registry + V1-hook `main_ppo`). This adapter is written against that shape. ## E2E smoke test (Golden Run 1, GSM8K, Qwen3-1.7B, single H200) Ran the recipe launcher end-to-end with the plugin loaded solely via `VERL_USE_EXTERNAL_MODULES` + `hydra.searchpath` -- no verl-core patches, no launcher modifications beyond those in this PR. | step | MFU (actor) | throughput (tok/s) | step time (s) | |---:|---:|---:|---:| | 1 | 0.261 | 5164 | 48.4 | | 2 | 0.256 | 5860 | 42.4 | | 3 | 0.260 | 6283 | 39.0 | | 4 | 0.264 | 6478 | 38.4 | Validation at step 4 (`val-core/openai/gsm8k/acc/mean@1`): 0.00152 (near-zero as expected for a 4-step GRPO run from a Qwen3-1.7B baseline; the goal here is to exercise the eval path, not to converge). Weight sync runs over CUDA-IPC and the receiver logs `[weight-sync names validated] context=cuda_ipc sender=310 expected=310` on every step. No tracebacks; clean vLLM engine shutdown. Co-authored-by: Cursor <cursoragent@cursor.com>
sfc-gh-kganesan
added a commit
to Snowflake-AI-Research/Arctic-Platform
that referenced
this pull request
Jul 10, 2026
…form/integrations/verl/ Ships the ArcticRL <-> verl adapter as an opt-in subpackage under `arctic_platform/integrations/`, plugged into verl at runtime via the `VERL_USE_EXTERNAL_MODULES` hook. Closes #35. ## Why verl core keeps its generic `RemoteBackend` ABC + registry; all Arctic-specific runtime code (client wrapper, per-backend worker, `RolloutReplica`, Hydra config, verl-shaped GRPO loss) now lives in one place inside `arctic_platform` and is discovered by verl at bootstrap. No verl source-tree patches, no forked launcher, no eager DeepSpeed / vLLM / arctic_inference imports for jobs that don't select the arctic backend. ## What lands New subpackage `arctic_platform.integrations.verl/`: | file | role | |---|---| | `register.py` | Loaded by `VERL_USE_EXTERNAL_MODULES`; eagerly imports `adapter` so `@RemoteBackendRegistry.register("arctic")` runs, then registers lazy loaders for the actor-rollout worker (`RemoteBackendRegistry.register_worker`) and the rollout replica (`RolloutReplicaRegistry.register`). | | `adapter.py` | `ArcticRLClientWrapper` -- verl's `RemoteBackend` ABC on top of `arctic_platform.rl`. | | `worker.py` | `ArcticRLActorRolloutRefWorker` -- CPU-only forwarder verl instantiates for the `ActorRollout(Ref)` role. | | `rollout.py` | `ArcticReplica` / `ArcticLLMServer` -- hosts Arctic's vLLM engine as a verl `RolloutReplica`. | | `grpo_loss.py` | Server-side verl-shaped GRPO loss (registered as `"verl_grpo"` on `arctic_platform.rl.processors.LOSS_FNS`). | | `config/remote_backend/arctic.yaml` | Per-backend Hydra config block; loaded via `remote_backend=arctic` with `hydra.searchpath` pointing at this dir. | | `examples/run_{gsm8k,bird}_grpo_arl.sh` | Reference single-GPU GRPO launchers matching Golden Runs 1 & 2. | Backward compatibility: `arctic_platform/rl/processors/verl_grpo.py` is now a one-line wildcard re-export of the new module, so the old import path (and its `LOSS_FNS["verl_grpo"]` side effect) still works. `pyproject.toml` gets a new `[verl]` extra and includes the plugin's yaml / example / README assets in the wheel. ## Recipe alignment `recipes/rl/verl/simple/`: - `run_qwen3_1.7b_gsm8k_grpo_arl.sh` now exports `VERL_USE_EXTERNAL_MODULES=arctic_platform.integrations.verl.register` and passes `hydra.searchpath=[file://.../integrations/verl/config]` to `verl.trainer.main_ppo`. No other launcher changes. - README clarifies that the Arctic backend ships as a plugin and that verl core carries no Arctic-specific files. ## Tests `tests/integrations/verl/` -- 17 tests, all green, all import-safe on machines without vLLM / DeepSpeed / arctic_inference (`conftest.py` stubs `verl.*`, `arctic_platform.rl.*`, and heavy transitive deps before importing the plugin): - `test_register.py`: decorator-based backend registration side effect, lazy worker + replica registration, loader-target verification via `inspect.getsource`. - `test_adapter.py`: `ArcticRLClientWrapper` API surface + `destroy()` idempotency. - `test_payload.py`: `_prepare_padded_arctic_batch_dict` shape / dtype invariants over nested-tensor inputs. - `test_backward_compat.py`: the shim re-exports the same function object and preserves the `LOSS_FNS["verl_grpo"]` registration. ## Companion verl-core change Paired PR: verl-project/verl#6422 (generic `RemoteBackend` ABC + registry + `register_worker` / `get_worker` on the registry + V1-hook `main_ppo`). This adapter is written against that shape. ## E2E smoke test (Golden Run 1, GSM8K, Qwen3-1.7B, single H200) Ran the recipe launcher end-to-end with the plugin loaded solely via `VERL_USE_EXTERNAL_MODULES` + `hydra.searchpath` -- no verl-core patches, no launcher modifications beyond those in this PR. | step | MFU (actor) | throughput (tok/s) | step time (s) | |---:|---:|---:|---:| | 1 | 0.261 | 5164 | 48.4 | | 2 | 0.256 | 5860 | 42.4 | | 3 | 0.260 | 6283 | 39.0 | | 4 | 0.264 | 6478 | 38.4 | Validation at step 4 (`val-core/openai/gsm8k/acc/mean@1`): 0.00152 (near-zero as expected for a 4-step GRPO run from a Qwen3-1.7B baseline; the goal here is to exercise the eval path, not to converge). Weight sync runs over CUDA-IPC and the receiver logs `[weight-sync names validated] context=cuda_ipc sender=310 expected=310` on every step. No tracebacks; clean vLLM engine shutdown. Co-authored-by: Cursor <cursoragent@cursor.com>
sfc-gh-kganesan
added a commit
to Snowflake-AI-Research/Arctic-Platform
that referenced
this pull request
Jul 10, 2026
…form/integrations/verl/ Ships the ArcticRL <-> verl adapter as an opt-in subpackage under `arctic_platform/integrations/`, plugged into verl at runtime via the `VERL_USE_EXTERNAL_MODULES` hook. Closes #35. ## Why verl core keeps its generic `RemoteBackend` ABC + registry; all Arctic-specific runtime code (client wrapper, per-backend worker, `RolloutReplica`, Hydra config, verl-shaped GRPO loss) now lives in one place inside `arctic_platform` and is discovered by verl at bootstrap. No verl source-tree patches, no forked launcher, no eager DeepSpeed / vLLM / arctic_inference imports for jobs that don't select the arctic backend. ## What lands New subpackage `arctic_platform.integrations.verl/`: | file | role | |---|---| | `register.py` | Loaded by `VERL_USE_EXTERNAL_MODULES`; eagerly imports `adapter` so `@RemoteBackendRegistry.register("arctic")` runs, then registers lazy loaders for the actor-rollout worker (`RemoteBackendRegistry.register_worker`) and the rollout replica (`RolloutReplicaRegistry.register`). | | `adapter.py` | `ArcticRLClientWrapper` -- verl's `RemoteBackend` ABC on top of `arctic_platform.rl`. | | `worker.py` | `ArcticRLActorRolloutRefWorker` -- CPU-only forwarder verl instantiates for the `ActorRollout(Ref)` role. | | `rollout.py` | `ArcticReplica` / `ArcticLLMServer` -- hosts Arctic's vLLM engine as a verl `RolloutReplica`. | | `grpo_loss.py` | Server-side verl-shaped GRPO loss (registered as `"verl_grpo"` on `arctic_platform.rl.processors.LOSS_FNS`). | | `config/remote_backend/arctic.yaml` | Per-backend Hydra config block; loaded via `remote_backend=arctic` with `hydra.searchpath` pointing at this dir. | | `examples/run_{gsm8k,bird}_grpo_arl.sh` | Reference single-GPU GRPO launchers matching Golden Runs 1 & 2. | Backward compatibility: `arctic_platform/rl/processors/verl_grpo.py` is now a one-line wildcard re-export of the new module, so the old import path (and its `LOSS_FNS["verl_grpo"]` side effect) still works. `pyproject.toml` gets a new `[verl]` extra and includes the plugin's yaml / example / README assets in the wheel. ## Recipe alignment `recipes/rl/verl/simple/`: - `run_qwen3_1.7b_gsm8k_grpo_arl.sh` now exports `VERL_USE_EXTERNAL_MODULES=arctic_platform.integrations.verl.register` and passes `hydra.searchpath=[file://.../integrations/verl/config]` to `verl.trainer.main_ppo`. No other launcher changes. - README clarifies that the Arctic backend ships as a plugin and that verl core carries no Arctic-specific files. ## Tests `tests/integrations/verl/` -- 17 tests, all green, all import-safe on machines without vLLM / DeepSpeed / arctic_inference (`conftest.py` stubs `verl.*`, `arctic_platform.rl.*`, and heavy transitive deps before importing the plugin): - `test_register.py`: decorator-based backend registration side effect, lazy worker + replica registration, loader-target verification via `inspect.getsource`. - `test_adapter.py`: `ArcticRLClientWrapper` API surface + `destroy()` idempotency. - `test_payload.py`: `_prepare_padded_arctic_batch_dict` shape / dtype invariants over nested-tensor inputs. - `test_backward_compat.py`: the shim re-exports the same function object and preserves the `LOSS_FNS["verl_grpo"]` registration. ## Companion verl-core change Paired PR: verl-project/verl#6422 (generic `RemoteBackend` ABC + registry + `register_worker` / `get_worker` on the registry + V1-hook `main_ppo`). This adapter is written against that shape. ## E2E smoke test (Golden Run 1, GSM8K, Qwen3-1.7B, single H200) Ran the recipe launcher end-to-end with the plugin loaded solely via `VERL_USE_EXTERNAL_MODULES` + `hydra.searchpath` -- no verl-core patches, no launcher modifications beyond those in this PR. | step | MFU (actor) | throughput (tok/s) | step time (s) | |---:|---:|---:|---:| | 1 | 0.261 | 5164 | 48.4 | | 2 | 0.256 | 5860 | 42.4 | | 3 | 0.260 | 6283 | 39.0 | | 4 | 0.264 | 6478 | 38.4 | Validation at step 4 (`val-core/openai/gsm8k/acc/mean@1`): 0.00152 (near-zero as expected for a 4-step GRPO run from a Qwen3-1.7B baseline; the goal here is to exercise the eval path, not to converge). Weight sync runs over CUDA-IPC and the receiver logs `[weight-sync names validated] context=cuda_ipc sender=310 expected=310` on every step. No tracebacks; clean vLLM engine shutdown. Co-authored-by: Cursor <cursoragent@cursor.com>
…end-plugin-hook [trainer, remote_backend] refactor: make RemoteBackend adapters plugin-only; strip Arctic from verl core
|
@wuxibin89 can you please a take a look? Follow-up to the RemoteBackend abstraction (verl-project/verl#6422). Turns the Arctic-aware verl core into a plugin-only shape so adapters live entirely out-of-tree via the existing Why
What lands
Net: +144 / −1682 across 15 files. Companion Arctic-Platform PRPaired plugin ships as Snowflake-AI-Research/Arctic-Platform#36 — Pre-commitFull verl pre-commit runs clean: Smoke testsBoth Arctic-Platform Golden Runs pass end-to-end against this tree + the paired plugin, using the shipped recipe launchers unchanged. Golden Run 1 — GSM8K (Qwen3-1.7B, 1×H200, 4 steps)
Weight sync validated every step ( Golden Run 2 — BIRD text-to-SQL (Qwen3-0.6B, 1×H200, 20 steps) Training reward (
Reward 0.335 → 0.415 while response length drops 1116 → 920. Validation @ step 20 vs pre-plugin reference log:
Follow-ups (not in this PR)
|
wuxibin89
approved these changes
Jul 16, 2026
4 tasks
sfc-gh-kganesan
added a commit
to Snowflake-AI-Research/Arctic-Platform
that referenced
this pull request
Jul 30, 2026
…nges Both merged upstream integrations (NovaSky-AI/SkyRL#1837:integrations/arctic_rl/ and the Arctic-specific RemoteBackend used by verl-project/verl#6422 living at arctic_platform/integrations/verl/adapter.py) construct their client with from arctic_platform.rl import ArcticRLClientConfig, create_arctic_rl_client client = create_arctic_rl_client(config, server_state) The goal is Cortex serverless support without either integration changing. This commit adds Cortex as a first-class backend on that single entry point. ## What ### `arctic_platform/rl/config.py` - `backend` Literal widened `["local", "dss-platform"]` -> add `"cortex"`. - Cortex-only fields added: `cortex_host` / `_database` / `_schema` / `_endpoint` / `_pat_env_var` / `_base_url`, `max_seq_len`. All Optional; missing keys fall back to `CortexTransport` defaults / `CORTEX_*` env vars. - `_derive_host_port` short-circuits on `backend == "cortex"` (Cortex has no local host/port; SnowAPI routing is by base_url). - `_validate_local_gpu_counts` already scoped to `backend == "local"`; unchanged. ### `arctic_platform/rl/client.py` - `create_arctic_rl_client(config, server_state)` early-branches when `config.backend == "cortex"` and returns `_CortexClientShim` via `_cortex_dispatch.build_cortex_client`. - `ArcticRLHTTPClient` / `ArcticRLRayClient` imports moved *inside* the http / ray branches so the Cortex code path never triggers the on-prem server dep chain (uvicorn -> arctic_inference -> vllm). The type hint for `ArcticRLServerState` is behind `TYPE_CHECKING`. ### `arctic_platform/rl/_cortex_dispatch.py` (new) - `_to_unified_config(legacy)` builds an `arctic_platform.client.ArcticRLClientConfig` sized for Cortex: backend, model_name, GPU counts, seed, all seven cortex_* / max_seq_len fields, and reconnect job ids. On-prem-only knobs (`ds_config`, `arctic_inference_config`, `ds_worker_config`, `log_prob_ds_config`, `full_determinism`, `colocate`, etc.) are intentionally NOT forwarded — Cortex has no local placement to configure and silently passing them would mask real config drift. - `_CortexClientShim` wraps `arctic_platform.client.ArcticRLClient` (which is synchronous by design) and re-exposes the legacy async surface both integrations reach for: `async def fwd_bwd / fwd_no_grad / step / save_checkpoint / save_weights / generate / sync_weights / reset_prefix_cache / wake_/sleep_ × {training, inference, log_prob} / empty_training_cache / weight_norm / log_probs`. Sync methods (`reconnect_config`, `get_server_state`, `shutdown`) match the legacy signature. Properties `config` / `training_job_id` / `sampling_job_id` / `log_prob_job_id` pass through. - `reconnect_config()` round-trips through the legacy config with cortex job ids populated so verl's `reconnect_handle()` pattern keeps working. - `get_server_state()` returns `None` (Cortex has no local Ray state actor; verl's reconnect path re-attaches via `training_job_id`). ### `arctic_platform/rl/__init__.py` - Lazy-load the heavy exports via PEP 562 `__getattr__`. - Eager: `ArcticRLClientConfig`, `WeightSyncConfig` (pydantic-only). - Lazy: `create_arctic_rl_client`, `WeightSyncCoordinator`, and all six `processors.*` re-exports. First attribute access resolves + caches in `globals()` so subsequent lookups hit the normal fast path. - Rationale: prior to this, `from arctic_platform.rl import ArcticRLClientConfig` on a Cortex-only driver pulled `arctic_platform.rl.client` -> `http_client` -> `http_server` -> `arctic_inference.server.metrics` -> `vllm` at package-init time. The Cortex dispatch never actually executes any of that; the eager import was pure overhead that forced Cortex users to install the entire on-prem ML stack. Post-refactor, a Cortex driver needs `arctic-platform` + `pydantic` + `requests`. Nothing else. ### `tests/client/test_rl_cortex_dispatch.py` (new, 23 tests) - `TestConfigTranslation`: cortex backend translates 1:1; cortex fields threaded; missing fields fall back to unified defaults; on-prem fields NOT forwarded (silent-drift guard); reconnect job ids forwarded. - `TestShimAsyncSurface`: pins every async call site both integrations hit — `fwd_bwd` / `fwd_no_grad` w/ `reference_model=` + `post_processors=`, `step(learning_rate=...)`, `sync_weights(cuda_ipc, low_memory)`, `wake_inference(tags=...)`, `sleep_inference(level=...)`, full colocation lifecycle, `generate(prompts, sampling_params)`, `save_checkpoint(stage_info, path)`. - `TestShimSyncSurface`: `shutdown()` is sync; `reconnect_config()` returns a `_LegacyConfig` with cortex job ids populated; `get_server_state()` returns `None` on Cortex. - `TestPropertySurface`: `client.config` returns the legacy config instance (SkyRL reads `client.config.colocate`); job-id properties pass through. - `TestFactoryDispatch`: - `test_backend_cortex_returns_shim`: `create_arctic_rl_client` picks the Cortex path and translates the config correctly. - `test_cortex_path_never_imports_onprem_transports`: pins the invariant — after a cortex dispatch, `arctic_platform.rl.http_*` / `ray_*` modules are NOT in `sys.modules`. Regression here means Cortex drivers pay the on-prem dep cost again. - `test_onprem_transports_are_lazy_at_module_level`: pins the source- level invariant — `arctic_platform.rl.client` module must NOT expose `ArcticRLHTTPClient` / `ArcticRLRayClient` as attributes. ## Result - 92 tests pass locally (69 previously-existing + 23 new). Test file runs on any machine — no vllm, no ray, no arctic_inference required. - `from arctic_platform.rl import ArcticRLClientConfig` on a bare pydantic+requests env loads exactly 2 modules and 0 heavy deps. - Neither upstream integration needs any code change to gain Cortex support; flipping `config.backend = "cortex"` + populating `cortex_*` fields is the entire user-facing UX. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Hi @sfc-gh-truwase, thanks for the RemoteBackend abstraction! We run a dual-external setup (external Megatron training + external SGLang, verl as a GPU-less orchestrator). Since the adapter landed in If migrating this to the V1 trainer isn't on your roadmap right now, could we help with that upstream? Happy to share our branch and align on the design first. Thanks! |
Author
|
@KunWuLuan thanks for sharing your work and offering to collaborate. We are interested. Can you review how close #7102 is to what your branch/design? |
Contributor
|
Thanks @sfc-gh-truwase! We compared #7102 against our port in detail — short answer: #7102 is sufficient for us. The contract layer (RemoteBackend ABC + registry) is effectively identical to what we carry, and its V1 wiring (extension hooks, CPU-only pool, weight-sync short-circuit) covers everything our trainer-side port does. Once it lands, we'll retire our port and the inline fallback entirely, keeping only the plugin itself (external Megatron + external SGLang) wired in via register_worker. Thanks again — and happy to help with the rebase or run an independent e2e check in our cross-cluster setup if useful. |
Author
|
@KunWuLuan thanks for the confirmation. I will continue our chat on #7102 |
sfc-gh-kganesan
added a commit
to Snowflake-AI-Research/verl
that referenced
this pull request
Aug 19, 2026
Extends the V0 RemoteBackend seam (verl-project#6422) to the V1 trainer stack so out-of-process RL backends (e.g. Arctic-Platform) can plug into V1 without touching verl's V0 or V1 hot paths. verl-core: * verl/remote_backend/{base,worker_utils,__init__}.py: RemoteBackend ABC + registry (name -> backend class + lazy forwarder loader) and backend-agnostic tensor/metric helpers. Ported from V0 unchanged. * verl/checkpoint_engine/remote_backend.py: no-op CheckpointEngine registered under 'remote_backend'. send/receive are never called on the short-circuit path. * verl/checkpoint_engine/{__init__,base}.py: eager import above + CheckpointEngineManager.update_weights short-circuit for backend='remote_backend' with set_global_steps fan-out to replicas that implement it. * verl/trainer/ppo/v1/trainer_base.py: three PPOTrainer extension hooks (_actor_rollout_wg_extra_kwargs, _llm_server_replica_init_kwargs, _checkpoint_engine_backend). Defaults preserve existing V1 behavior. * verl/trainer/ppo/v1/trainer_remote_backend.py: PPOTrainerRemoteBackend (subclass of PPOTrainerSync). Builds the plugin's RemoteBackend on the driver, smuggles reconnect_handle to forwarder workers and rollout replicas, marks the resource pool CPU-only, and asserts single-forwarder when the backend requires it. * verl/trainer/ppo/v1/__init__.py: re-export. * verl/trainer/config/ppo_trainer.yaml: 'optional remote_backend@remote_backend: null' defaults entry. * verl/trainer/main_ppo.py: _resolve_remote_backend_from_hydra_choice mirrors the 'remote_backend=<name>' Hydra choice onto trainer.remote_backend and trainer.v1.trainer_mode. * verl/workers/rollout/{replica,llm_server}.py: RolloutReplica accepts **kwargs; LLMServerManager accepts replica_init_kwargs and forwards to replica constructors. * verl/single_controller/ray/base.py: ResourcePoolManager.use_gpu (default True); CPU-only pools skip the GPU headroom check. * tests/remote_backend/: 9 CPU-only tests (registries, trainer dispatch, Hydra choice hook). Validation: 8B BIRD text-to-SQL, recipe-aligned, 5 steps. update_actor 380.9s -> 166.0s (2.30x from ZoRRo), total step 731.4s -> 438.3s (1.67x), convergence preserved (reward means match step-over-step). Net diff: +119 / -6 lines in existing files, ~460 new lines in the plugin-seam code and tests. Co-authored-by: Cursor <cursoragent@cursor.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
9 participants
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?
Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI){modules}includefsdp,megatron,veomni,sglang,vllm,rollout,trainer,ci,training_utils,recipe,hardware,deployment,ray,worker,single_controller,misc,perf,model,algo,env,tool,ckpt,doc,data,cfg,reward,fully_async,one_step_off,like[megatron, fsdp, doc]{type}is infeat,fix,refactor,chore,test[BREAKING]to the beginning of the title.[BREAKING][fsdp, megatron] feat: dynamic batchingTest
API and Usage Example
# Add code snippet or script demonstrating how to use thisDesign & Code Changes
Checklist Before Submitting
Important
Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=alwaysci-requestchannel in theverlSlack workspace. (If not accessible, please try the Feishu group (飞书群).)recipesubmodule, please also update the reference to the submodule commit viagit submodule update --remoteorcd recipe && git pull origin main.