Skip to content

Initialize Miles LoRA A matrices with kaiming-uniform like the megatron backend - #10

Draft
kevintli wants to merge 1 commit into
devin/1790764407-execute-sample-target-inputsfrom
devin/1790783613-miles-lora-kaiming-init
Draft

kevintli wants to merge 1 commit into
devin/1790764407-execute-sample-target-inputsfrom
devin/1790783613-miles-lora-kaiming-init

Conversation

@kevintli

@kevintli kevintli commented Sep 30, 2026 •

Copy link
Copy Markdown

Summary

The Miles backend never sets args.lora_A_init_method, so Miles' create_multi_lora_instance falls back to getattr(args, "lora_A_init_method", "xavier"). In the bundled Megatron-Bridge MultiLoRA layers, "xavier" means xavier_normal_. Spindle's own megatron LoRA backend (megatron_runtime/lora/model.py) passes lora_A_init_method="kaiming" (kaiming_uniform_(a=sqrt(5)), the PEFT default). Thinking Machines' "LoRA Without Regret" post says Tinker uses that PEFT parametrization: uniform A with scale 1/sqrt(d_in), zero B, alpha 32.

# MilesRuntime._start, after parse_args
args.lora_A_init_method = "kaiming"

Why it matters: B starts at zero, and Adam's per-step update to B is about lr regardless of scale. So the early weight change ΔW = (α/r)·ΔB·A scales with std(A), and xavier gives a higher effective LR than Tinker at the same learning_rate.

Measured std(A) in the exported step-0 adapters for gpt-oss-20b (layer 3 and lm_head). The PEFT reference is 1/sqrt(3·d_in):

target xavier (before) kaiming (this PR) PEFT per-matrix
q/k/v_proj, lm_head (d_in 2880) 0.0262 0.0108 0.0108
o_proj (d_in 4096, TP-sharded) 0.0605 0.0255 0.0090
experts gate_up/down (packed 3D) 0.0044 0.0019 0.0108

This PR fixes the dense q/k/v/lm_head targets exactly. Two gaps remain because the init runs on local, packed tensors, which gives the wrong fan-in:

  • Row-parallel o_proj sees the 512-wide TP shard.
  • Packed experts [local_experts, rank, in] see rank*in.

Fixing those needs per-expert, global-fan-in init in Megatron-Bridge. That is out of scope here; this PR only makes the Miles path consistent with Spindle's megatron path.

Found while reproducing jasper-lu/sec-search-rl, where Spindle policies collapsed to short trajectories faster than the hosted-Tinker reference.

Link to Devin session: https://modal.devinenterprise.com/sessions/f53cfabb210146de8f0338fe388d7973
Open in Devin Desktop: https://modal.devinenterprise.com/desktop/session/f53cfabb210146de8f0338fe388d7973?variant=devin
Requested by: @kevintli

@devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration
devin-ai-integration Bot force-pushed the devin/1790783613-miles-lora-kaiming-init branch from 7f60417 to 216ab5a Compare September 30, 2026 20:25
…on backend

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
devin-ai-integration Bot force-pushed the devin/1790783613-miles-lora-kaiming-init branch from 216ab5a to 0ac5751 Compare September 30, 2026 20:48
@kevintli
kevintli added this pull request to stack #13 September 30, 2026 22:15
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