Skip to content

fix(lora): let a resumed disaggregated run reach the rollout engine - #2708

Open
Arist12 wants to merge 3 commits into
radixark:mainfrom
Arist12:fix/lora-disaggregated-resume-adapter
Open

fix(lora): let a resumed disaggregated run reach the rollout engine#2708
Arist12 wants to merge 3 commits into
radixark:mainfrom
Arist12:fix/lora-disaggregated-resume-adapter

Conversation

@Arist12

@Arist12 Arist12 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Part of #2705.

Problem

A disaggregated LoRA run resumed with --lora-adapter-path can fail before its first rollout: native-only checkpoints cannot be preloaded by SGLang, partial best-effort PEFT output can look preloadable, and a preloaded adapter collides with the mandatory initial publish.

Change

Normal train drivers start resumed rollout engines from the base model and let the mandatory initial weight publish install the trainer's restored adapter. Publish single LoRA adapters with SGLang's existing upsert=True contract so fresh startup and later refreshes use one path without unload churn. --debug-rollout-only, which skips the publish, remains a blocker before this PR is ready.

Validation

  • 28 focused fast tests covering complete, partial and native-only resume directories
  • Full Qwen3-30B-A3B, 4 trainer + 4 rollout MI350X: restored four EP shards (384 tensors each), optimizer, scheduler and global-dataset cursor from iter_29; continued at rollout 30, completed two optimizer steps, saved iter_30, and republished the trained adapter (grad_norm=0.0152, train/rollout abs diff 0.0210-0.0237)
  • Full Qwen3-4B, 4+4 MI350X: a 30-rollout run resumed at iter_4 and iter_14, restored 288 tensors plus optimizer/scheduler/data cursor, and completed through iter_29
  • Dense held-out DAPO accuracy 0.711 → 0.789, truncation 0.211 → 0.078; train/rollout abs diff mean 0.01265, max 0.01419
  • Checkpoints at 4/14/29 each contain one finite TP0/PP0 native shard and distinct adapter hashes

Resuming a disaggregated LoRA run with --lora-adapter-path fails in two
places, so the save/resume pair only ever worked colocated.

SGLang is asked to preload the adapter directory whenever the flag is set.
The "dummy base load" escape hatch never fires, because nothing sets
MILES_SGLANG_DUMMY_LOAD=1. A LoRA checkpoint does not always contain an
adapter_config.json: the HF PEFT export is best-effort and Bridge declines
some MoE adapter layouts, leaving only the Megatron-native shards that the
trainer resumes from. The engine then aborts at startup on the missing file.
Preload only a directory SGLang can read; the adapter still arrives through
the weight sync that train.py and train_async.py both run before the first
rollout, carrying its own config_dict.

When the preload does succeed, the distributed updater starts with
_lora_loaded=False and skips the unload, but its load is not an upsert, so
SGLang rejects the duplicate adapter name on the first sync. Always attempt
the unload and tolerate its absence, matching UpdateWeightFromTensor.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Use SGLang's existing upsert contract so fresh and preloaded adapters follow one path without failed unloads or repeated slot churn.
Use the mandatory initial trainer publish as the single startup path so partial PEFT exports and native-only checkpoints cannot diverge before the first rollout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant