Skip to content

[multi-lora] skip pause/resume for lora with merge=false#1677

Merged
erictang000 merged 7 commits into
NovaSky-AI:mainfrom
erictang000:lora_skip_pause
May 16, 2026
Merged

[multi-lora] skip pause/resume for lora with merge=false#1677
erictang000 merged 7 commits into
NovaSky-AI:mainfrom
erictang000:lora_skip_pause

Conversation

@erictang000
Copy link
Copy Markdown
Collaborator

@erictang000 erictang000 commented May 16, 2026

pausing and resuming generation are not needed when updating lora weights in vLLM - we can simply rely on load_lora_adapter (https://docs.vllm.ai/en/stable/features/lora/#in-place-lora-reloading) to directly update the weights in the engine without worrying about corrupted weight state.

Results for fully async RL with megatron + vllm LoRA - we can see that removing the pause/resume for the merge lora = false case still works and has an identical reward curve!

image

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a training script for fully async GRPO on GSM8K with Megatron and LoRA. It also updates the weight synchronization logic to avoid pausing generation when using non-merged LoRA. Feedback indicates that the condition for skipping the pause is too broad and could cause errors on non-Megatron strategies; a more robust implementation using getattr and a backend check for vLLM is recommended.

Comment thread skyrl/backends/skyrl_train/workers/worker_dispatch.py Outdated
erictang000 added a commit that referenced this pull request May 16, 2026
…1678)

Reverts #1657, since we can skip pause/resume for
multi-tenant lora after #1677
@erictang000 erictang000 merged commit 654d7ab into NovaSky-AI:main May 16, 2026
4 of 5 checks passed
@erictang000 erictang000 deleted the lora_skip_pause branch May 16, 2026 21:55
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