[lora] refactor - native LoRA - #2017
Open
yushengsu-thu wants to merge 14 commits into
Open
Conversation
Moves miles/backends/megatron_utils/lora_native.py (906 lines, one file)
into a miles_plugins/lora package, so model-architecture support lives in
the plugin layer like mbridge and megatron_bridge already do. Bridge-mode
LoRA is untouched: the shared helpers both paths use stay in
lora_utils.py, and nothing under miles/ imports the plugin at module
level -- core reaches it only through the resolve_lora_provider dotted
path, whose default is now miles_plugins.lora.lora.
Layout, split along the seams the code already had:
* adapter.py -- NativeLoRAAdapter and the _ADAPTER_LAYOUT projection
table. Adapters stay self-describing, so export/load
never know which spec attached them.
* io.py -- HF/PEFT export (TP-gathered) and per-rank load.
* naming.py -- the checkpoint-index naming heuristic, plus an optional
mbridge cross-check (lazy import, warns on disagreement,
no-op when mbridge is absent).
* spec/ -- attach paths: attention.py (GQA fused-qkv + MLA, with
their guards), mlp.py (gated MLP / shared experts),
base.py (per-run _Spec and shared primitives).
* registry.py -- new: HF model_type -> spec family, keyed the same way
mbridge's register_model is. Unregistered architectures
fail at startup listing the supported set and the
escape hatches; checkpoints with no config.json (the
numerical harnesses) fall back to structural dispatch
with a warning. The config x parallelism guards
(num_query_groups >= TP, MLA q_lora_rank, replicated
down-projections) are unchanged and still apply on top.
* lora.py -- the provider entrypoint: apply_native_lora, the run
guards, and the three-function provider protocol.
Function bodies are unchanged from lora_native.py except apply_native_lora
gaining the registry lookup and arch=... in its startup log line.
resolve_lora_provider moves to lora_utils.py, next to the other helpers
its call sites (model.py, hf_weight_iterator_direct.py,
save_lora_checkpoint) already import.
Tests move to tests/fast/miles_plugins/lora/ -- the 47 existing tests
unchanged apart from imports, plus 17 new registry/naming tests. Verified
against the parent branch in the same environment: identical pass/fail
sets on the moved suite and on the four lora_utils-dependent suites
(test_lora_utils, test_lora_model_branches, test_lora_update_weight,
test_lora_hf_weight_iterator); the only local failure either way is
megatron-core's triton import on macOS. black / isort / ruff clean.
Co-authored-by: Cursor <cursoragent@cursor.com>
yushengsu-thu
requested review from
Shi-Dong,
Zhichenzzz,
fzyzcjy,
guapisolo,
jybsuper,
maocheng23 and
yueming-yuan
as code owners
July 31, 2026 02:57
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
yushengsu-thu
commented
Jul 31, 2026
…fety, API cleanup - MoE without shared experts (qwen3_moe): parser-expanded all-linear MLP targets now skip with a log line instead of asserting at startup, mirroring the MLA generic-qkv normalization; explicit gate/up/down requests still fail closed. - Registry: drop deepseek_v4 (DeepSeek-V4-Flash attention is wq_a/wq_b/wkv, not mcore MLA — docs already declare it out of scope), and report "config.json has no model_type" distinctly from "config.json missing". - Resume safety: load_native_adapter_state_dict now reports missing tensors (shard saved with narrower targets) in addition to unexpected ones, and the optimizer/scheduler-restore decision is agreed across ranks via all_gather_object so per-rank shard differences cannot desync the resume iteration or LR schedule. - Provider path: the package itself now exports the provider protocol, the default becomes miles_plugins.lora (old miles_plugins.lora.lora pins keep working, including for the native save fast path). - Delete all 17 transitional underscore aliases; tests and the numerical harness import the public names. - Extract parse_lora_target_modules out of miles_validate_args so tests/fast/utils/test_lora_arguments.py exercises the production function instead of a hand-copied replica (now also covers MLA all-linear expansion). - New tests/fast/miles_plugins/lora/test_orchestration.py: apply_native_lora attach counts / freeze / grad flow on fake decoders, registry-gated startup failures, _assert_supported_run rejections, and MLA attach for both q_lora_rank branches (previously 0% fast-test coverage on lora.py and the MLA attach path). - docs: VL wrappers resolve via text_config.model_type (vision never adapted; unregistered VL types fail closed), shared-expert-less MoE all-linear behavior, provider default.
yushengsu-thu
force-pushed
the
yusheng/lora-plugin-refactor
branch
from
July 31, 2026 20:11
f3c0c73 to
dd9d77c
Compare
- SharedExpertOnlyMoESpec splits into SharedOuterExpertMoESpec (layers with a shared/outer expert: LoRA adapts the shared expert's MLP) and GeneralExpertMoESpec (routed-only layers: all-linear skips, explicit MLP targets fail closed). The shared spec delegates routed-only layers to the general spec, so one registry entry still covers mixed-layer models. Module docstring spells out the distinction from the bridge-only --experts-shared-outer-loras adapter layout (SGLang PR #21466). - SplitQKV -> LoRASplitQKV, SplitFC1 -> LoRASplitFC1, aligning with Megatron-Bridge's LoRALinearSplitQKV / LoRALinearSplitFC1UpGate naming (LoRALinear already matches).
…pported combos
Findings from a parallelism audit of the native path. TP-only handling is
correct for what the plugin attaches (ETP is moot because MCore builds the MoE
shared expert on the attention TP group; CP needs nothing because adapters are
ordinary replicated DDP-bucket params reduced over dp_cp), but the surrounding
plumbing had two real bugs:
- EP>1 resume found no shard. The adapter shard name carried ep_rank while only
dense-DP-rank-0 ranks wrote, so under scripts/run_lora_native.py's shipped
TP2/EP4 defaults exactly {tp0_pp0.pt, tp1_pp0_ep1.pt} were written and the
EP 2/3 ranks found nothing (verified against the pinned fork's RankGenerator).
Native adapter state is EP-invariant, so its name is now keyed on (tp, pp)
only; bridge shards keep the EP component because bridge PEFT can attach
genuinely expert-parallel adapters. Writers are now elected per distinct
shard filename, which covers every name some rank will request under any
layout and collapses to the old behavior when the name has no EP component.
- --overlap-grad-reduce silently corrupted adapter gradients: the TP grad sum
runs before finalize_model_grads and writes the same main_grad buffer MCore's
per-bucket reduce-scatter is already streaming into. It cannot move after
finish_grad_sync (--use-distributed-optimizer leaves only the local shard
valid), so it is refused at startup.
Guards for combinations that silently did the wrong thing:
- --experts-shared-outer-loras now requires bridge mode or a custom provider;
native adapts no routed experts, so SGLang was being configured for buffers
the export never fills.
- --train-backend fsdp + --lora-rank now fails instead of silently
full-finetuning while the rollout engine expects an adapter.
- qwen3_5/qwen3_6/qwen3_next warn that raw mode's frozen-base backward is known
to diverge there (scripts/run_lora_native.py documents grad_norm 1e7-1e10 /
NaN), so a run says so instead of blowing up at step 1.
Also records, as roadmap notes next to the code that would change, the
dimensions Megatron-Bridge PEFT covers and native does not: routed-expert LoRA
in three layouts and router LoRA (spec/moe.py), the expert-TP group and the
structural reason overlap flags are refused (distributed.py), expert-axis
export (codec/hf.py), GDN in_proj and uncompressed-MLA q_proj (spec/attention.py),
each with the bridge file:line to port from.
create_lora_instance passed both target_modules and exclude_modules to the bridge LoRA, but Megatron-Bridge treats them as mutually exclusive selection mechanisms and asserts the latter is empty whenever the former is set (peft/module_matcher.py). Since --target-modules is required whenever LoRA is on, any --exclude-modules value made every bridge-mode LoRA run die during the module walk with "exclude_modules should be empty when using target_modules". Reproduced against the pinned radixark/Megatron-Bridge matcher. The forwarded value was redundant anyway: parse_lora_target_modules already subtracts the excluded names from --target-modules, which is the only mechanism both LoRA paths share, and miles can never reach the bridge branch that honors exclude_modules. So the kwarg is dropped (verified against the real matcher: linear_qkv still matches, the excluded linear_proj returns None, no assert), and parse_exclude_modules goes with it — its only caller was that kwarg. Wildcard excludes now fail at startup instead of being silently ignored: exact names are all the subtraction can remove, the native provider matches leaf names, and bridge refuses patterns alongside a target list.
yushengsu-thu
commented
Aug 1, 2026
Collaborator
Author
|
To-do
|
Applies the style already used in distributed.py and spec/moe.py to the places that still carried long roadmap prose: codec/hf.py, codec/checkpoint.py, modules/linear.py, modules/moe.py, spec/base.py, the GDN and MLA spec docstrings, the plugin __init__, five docstrings and one comment in lora_utils.py, parse_lora_target_modules, two test docstrings, and the docs parallelism section. Assertion and warning text is untouched: those are user-facing diagnostics where naming the reason and the escape hatch is the point. No behavior change; 177 LoRA fast tests pass.
…g/lora-plugin-refactor # Conflicts: # tests/fast/fixtures/generation_fixtures.py # tests/fast/fixtures/rollout_fixtures.py
… / kimi-k2.5-2layer (#2073)
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
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.
Refactor PR into #1792's branch (
zhichen/lora-native), so the native-LoRA extraction ships as a plugin from the start. Adapter math is unchanged — a side-by-side harness against the base monolith shows bit-identical adapter parameters, forward outputs, backward gradients, and HF exports for full-target GQA (TP=1, TP=2 ± SP) and MLA configs. Bridge-mode LoRA is untouched.Includes #2072: a function-level redesign on top of the extraction, aimed at one-declaration support for new models, plus the fixes found while validating four models end-to-end.
Layout
Design (from #2072)
Classes carry the taxonomy; every fact is declared once and derived everywhere else.
spec/layout.py): an architecture spec is a class holding aModuleLayouttable — which projections exist, on which physical linear, with which shard geometry. One sharedattach_layout()implements the existence-check → target-filter → guard → build → hook walk; the dim-resolver section is the only spec-side reader of MCore attribute names. A new layout is ~15 lines of facts.LayoutSpec→AttentionSpecBase→GQA/MLA/GDN,HybridGQAGDN(GQAAttentionSpec),SharedOuterExpertMoESpec(GeneralExpertMoESpec)):supported_targets, the canonical--target-modulesorder, and SGLang's fused-family expansion all derive from the layout declaration; a new architecture's fused groups reach serving-side target expansion automatically.ShardLayout/AttentionFamilystr-enums replace bare strings; no parallel name tables in the exporters.ModelEntry(spec, status, reason)withVALIDATED / STRUCTURAL / UNSTABLE: support status is one first-class field; UNSTABLE entries carry their reason and warn at attach.exports() -> ProjectionExport(withSGLangFusedGroupmetadata);serving.pyconsumes only that API.default_target_modules(hf_checkpoint)andpreflight_native_lora(...)(registry ∧ mbridge bridge ∧ model-args audit, no GPU) — launch scripts stop re-declaring per-family target strings and fail in seconds when a conversion bridge is missing.Fixes found by running four models natively (from #2072)
miles_plugins/mbridge/kimi_k25.py(new): raw-mode K2.5 conversion needs an mbridge bridge (bridge mode uses megatron.bridge's ownKimiK25VLbridge and never converts). ThinDeepseekV3Bridgesubclass:text_configdescent +language_model.prefix remap.megatron_to_hfkimi dispatch:_convert_to_hf_corematched onlykimi_k25, but the direct weight iterator passeskimik25config(dead branch); the converter also lacked the raw text-only provider's unprefixed top-level weight names.convert_kimi_int4_to_bf16.pystripsquantization_config: a surviving one sends SGLang through its CompressedTensors path, which serves the BF16 checkpoint with a degenerate context-free forward → garbage rollouts, train/rolloutlogprob_abs_diff~2.2. Verified by a megatron-vs-HF-vs-SGLang three-way logprob comparison.torch_dsa_topkclamps k to the key length: short sequences (<index_topkkeys) crashedtorch.topkin raw-mode log-prob compute.run_glm5_2_744b_a40b_lora_native.py(new): raw-mode GLM-5.2 launcher; its toy conversion must be single-rank (any PP split of the 5-layer toy starts a stage on a DSA skip layer).Validation
20-rollout native GRPO runs (wandb
ch271828n-team/miles_lora_native), acceptance ≈ 0.01 train/rolloutlogprob_abs_diff:qwen3_5_moeVALIDATED)tests/fast/miles_plugins/lora: 47 passed. Behavior-preservation smoke: a 2-rollout Qwen3-8B run reproduces the baseline's first-rollout logprob_abs_diff (0.00967 vs 0.00978). Adapter attribute names are pinned, so checkpoint keys are unchanged.