Arctic RL: VeRL integration - #1
Open
sfc-gh-truwase wants to merge 70 commits into
Open
Conversation
Signed-off-by: Stas Bekman <stas.bekman@snowflake.com>
Signed-off-by: Stas Bekman <stas.bekman@snowflake.com>
Signed-off-by: Stas Bekman <stas.bekman@snowflake.com>
Signed-off-by: Stas Bekman <stas.bekman@snowflake.com>
…nto tunji/arl_cleanup
sfc-gh-truwase
added a commit
that referenced
this pull request
May 20, 2026
…traction-public Generic RemoteBackend abstraction + Arctic adapter (parallel to PR #1)
sfc-gh-kganesan
added a commit
that referenced
this pull request
May 29, 2026
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>
sfc-gh-kganesan
added a commit
that referenced
this pull request
Jun 16, 2026
Self-review of PR #6 surfaced three real issues; all addressed here. Scope remains vendor-only (verl/workers/remote_client/arctic_rl.py + verl/trainer/config/remote_backend/arctic.yaml); core abstraction under verl/remote_backend/ is still diff-clean vs public/arctic_rl_share_v0.7.1. 1. (important) Thread logits_optimization / logits_optimization_peak _mem_size_in_gib / logits_compute_in_fp32 into _create_ds_worker _config so they actually take effect. The server's authoritative read path for these three knobs is arctic_platform/rl/deepspeed_worker.py lines 319-322, which reads them from `ds_worker_config` (server-side config dict), NOT from per-call request meta. Before this commit they were only forwarded in compute_log_prob.meta / update_actor.meta, so the server silently fell back to its hard-coded defaults ("none", 4, False). The shipped yaml values happen to match those defaults, so this bug was masked at the validated configs. But the moment someone sets logits_optimization=memory in the yaml to mitigate an OOM (see the 1.7B attempt #1 deadlock that needed gpu_memory_utilization =0.5 to fix), the yaml change would silently no-op without this fix. Added them to the `if self.zorro_train_enable` block to match the lifecycle of the existing zorro-only ds_worker_config fields (response_len, max_token_len, rollout_n, temperature). The per-call meta forwarding is intentionally kept so a future arctic_platform.rl release that adds a per-batch override path will just work without an adapter change. 2. (important) Document zorro_train_load_balancer as forward-compat. grep -r zorro_train_load_balancer in arctic_platform/rl/ returns zero hits — no server consumer for this key on the currently installed version. We continue to forward it (matches PR verl-project#37 wire format, so the server-side handler can land independently in a later ArcticInference-internal release — likely tied to PR verl-project#92's strict group-balanced routing work — without an adapter change). Added a TODO comment so the next reader knows it's intentionally a no-op today, not a wiring bug. 3. (nit) Document the deliberately-not-exposed sibling key logits_compute_from_fp32_inputs. The server has both `logits_compute_in_fp32` (output-side upcast, exposed) and `logits_compute_from_fp32_inputs` (input-side upcast, not exposed) at deepspeed_worker.py:321-322. The PR verl-project#37 adapter also only exposes the output-side knob and none of the recipe/rl -correctness configs need the input-side one, so we match that. Added a yaml + __init__ comment so a future reviewer doesn't think the missing sibling is an oversight. Validation: - import smoke: OK - yaml loads via OmegaConf, all 5 new fields parse correctly - _create_ds_worker_config now produces a dict containing all 3 logits keys (confirmed via inspect.getsource grep) - no new lints - live 1.7B convergence run (wandb xqj9pqbt) is on commit 9624948 and not affected by this change; will pick up the fix on its next launch 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
3 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?
Demonstrates Arctic RL integration into VeRL. Implemented against a local fork of tag v0.7.1