Skip to content

fix: emit PEFT-standard disk LoRA adapter keys so vLLM can load them (incl. #1577) - #1579

Open
sitabulaixizawaluduo wants to merge 1 commit into
mainfrom
fix/lora-disk-adapter-key-naming
Open

fix: emit PEFT-standard disk LoRA adapter keys so vLLM can load them (incl. #1577)#1579
sitabulaixizawaluduo wants to merge 1 commit into
mainfrom
fix/lora-disk-adapter-key-naming

Conversation

@sitabulaixizawaluduo

Copy link
Copy Markdown
Collaborator

Description

Fixes the disk-mode LoRA adapter save format so vLLM can load AReaL-trained
adapters again, and folds in the one-line assertion fix from #1577.

1. vLLM disk LoRA load regression (main fix)

test_examples.py::test_gsm8k_grpo_lora[vllm:d1-fsdp:d1] fails on main with:

ValueError: model.layers.0.mlp.down_proj.lora_A.default.weight is unsupported LoRA weight

Root cause: #1444 replaced the PEFT-standard save
(PeftModel.save_pretrained) with a hand-rolled _save_lora_to_hf in
areal/engine/fsdp_engine.py to avoid gathering the full model state dict
(OOM on 27B/35B). Its key cleanup only stripped the base_model.model.
prefix but kept the active-adapter segment .default, so on-disk keys became
...lora_A.default.weight. vLLM's parse_fine_tuned_lora_name /
check_unexpected_modules expects PEFT-serving-standard keys
(...lora_A.weight) and rejects the .default form, so the vLLM server fails
to launch and the example subprocess exits 1.

Fix (minimal): drop the .default segment instead of stripping the
prefix, reproducing the exact pre-#1444 PEFT-standard format
(base_model.model....lora_A.weight). This keeps #1444's per-parameter
unshard (OOM avoidance) intact.

Why SGLang is unaffected (verified, not assumed): both backends load the
same adapter directory via /load_lora_adapter (lora_path=str(meta.path));
only vLLM's parser is strict. The SGLang LoRA test passed with the
PEFT-standard format before #1444 (run 30074565394, 251s) and with the
.default format after it (current run, 245s), i.e. SGLang tolerates both.
This fix restores the format SGLang already passed with.

2. Rolls in #1577

tests/test_sglang_lora_unload.py: the /load_lora_adapter request became
best_effort=True, but the assertion still expected False. Updated to True.

Type of Change

  • Bug fix

Test commands executed

  • pre-commit run --files areal/engine/fsdp_engine.py tests/test_sglang_lora_unload.py — passed
  • pytest tests/test_sglang_lora_unload.py — 5 passed

Notes / skipped suites

  • test_gsm8k_grpo_lora[vllm] requires the multi-GPU GCP runner and cannot be
    run locally; it exercises this exact path and should be validated in CI.

… and align sglang best-effort unload test assertion (#1577)
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@sitabulaixizawaluduo sitabulaixizawaluduo added the safe-to-test Ready to run unit-tests in a PR. label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe-to-test Ready to run unit-tests in a PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant