Skip to content

[sglang-miles] Warn instead of silently dropping one-sided MoE expert LoRA targets - #32376

Merged
yushengsu-thu merged 1 commit into
sgl-project:sglang-milesfrom
yushengsu-thu:moe-lora-one-sided-targets
Jul 25, 2026
Merged

[sglang-miles] Warn instead of silently dropping one-sided MoE expert LoRA targets#32376
yushengsu-thu merged 1 commit into
sgl-project:sglang-milesfrom
yushengsu-thu:moe-lora-one-sided-targets

Conversation

@yushengsu-thu

@yushengsu-thu yushengsu-thu commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Motivation

A FusedMoE layer is only wrapped with LoRA when both gate_up_proj and down_proj are in the target modules, because the MoE-LoRA hooks inject the two deltas together. Targeting only one left the layer unwrapped with no diagnostic: the adapter's expert weights for the targeted projection were loaded into the memory pool but never applied. For RL rollouts that means the serving policy silently diverges from the trained one.

Modifications

  • Emit a one-shot warning naming the MoE module and the missing projection. Kept a warning rather than a startup error: adapters that only train the dense or shared-expert MLP legitimately target one projection and keep working unchanged.
  • Drop the always-true target-module re-check in update_lora_info — a FusedMoEWithLoRA only exists when init_lora_modules already found both projections targeted.

No behavioural change beyond the log line; no accuracy or performance impact.

Checklist

  • Format the code with pre-commit.
  • Tests: diagnostic-only change; existing LoRA tests cover both the wrapped and unwrapped paths.

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ❌ Run #30170262739
Latest PR Test (Extra): ❌ Run #30170262678

Copilot AI review requested due to automatic review settings July 25, 2026 03:21
@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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added the lora label Jul 25, 2026
@yushengsu-thu

Copy link
Copy Markdown
Collaborator Author

Verified on an 8x H200 devbox with this branch editable-installed:

test/registered/unit/lora/   (test_lora_upsert.py, test_mem_pool_ep_unit.py,
                              test_moe_lora_tail_stamp.py)      67 passed

pre-commit run --files python/sglang/srt/lora/lora_manager.py is clean, and the diff is 3 hunks with no incidental reformatting.

The wrap/skip decision itself is unchanged by this PR — the two reachable states (both expert projections targeted → FusedMoEWithLoRA; neither targeted → skip) behave exactly as before, and the third state (exactly one targeted) previously skipped silently and now logs once before skipping.

… LoRA targets

A FusedMoE layer is only wrapped when both gate_up_proj and down_proj are in the
target modules, because the MoE-LoRA hooks inject a delta after gate_up and after
down together. When only one of them was targeted the layer stayed unwrapped with
no diagnostic, so an adapter carrying expert weights for the targeted projection
had them loaded into the memory pool and then never applied — a serving/training
mismatch with no symptom.

Emit a one-shot warning naming the module and the missing projection. Adapters
that only train the dense or shared-expert MLP keep working, so this stays a
warning rather than a startup error.

Also drop the redundant target-module re-check in update_lora_info: a
FusedMoEWithLoRA only exists when init_lora_modules already found both
projections targeted.
@yushengsu-thu
yushengsu-thu force-pushed the moe-lora-one-sided-targets branch from 70cbe21 to f552db9 Compare July 25, 2026 18:45
@yushengsu-thu
yushengsu-thu merged commit da05e95 into sgl-project:sglang-miles Jul 25, 2026
69 of 79 checks passed
yueming-yuan pushed a commit that referenced this pull request Jul 29, 2026
… LoRA targets (#32376)

Carried onto v0.5.16. The target-module gate is kept as-is: v0.5.16 added an
is_shared_fused_moe arm to the FusedMoEWithLoRA branch, and dropping the
redundant re-check there (as the original commit did) would let a shared-MoE
module through unchecked, which is beyond this fix's intent.
yueming-yuan pushed a commit that referenced this pull request Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants