Skip to content

[sglang-miles] Fix p2p weight registry: draft worker overwrite, fused indexer mapping, registration retries - #35312

Open
yueming-yuan wants to merge 46 commits into
sglang-milesfrom
yueming/p2p-registry-fixes
Open

yueming-yuan wants to merge 46 commits into
sglang-milesfrom
yueming/p2p-registry-fixes

Conversation

@yueming-yuan

@yueming-yuan yueming-yuan commented Aug 18, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes found while verifying miles p2p (RDMA) weight updates on GLM-5.2 744B fp8 with MTP enabled. Companion miles PR: radixark/miles#2605.

Worker-keyed engine-info bootstrap registry (engine_info_bootstrap_server.py, remote_instance_weight_transporter.py, model_runner.py, http_server.py)
With speculative decoding, the draft worker's ModelRunner.initialize() also registers with the EngineInfoBootstrapServer. The registry was keyed by tp_rank alone, last write wins, and the draft registers after the target — so the p2p client received the draft model's weight table and RDMA session for every rank: ordinary target params appeared missing on the remote and every write bucket failed. The registry key is now (worker, tp_rank) (worker in {"target", "draft"}), the transporter tags its registrations, and the two query endpoints take an optional worker parameter defaulting to "target" — existing clients keep their behavior, and a p2p client can address the draft explicitly. Long-running MTP training needs exactly that: the trained nextn layer must reach the draft, or speculative acceptance decays to zero as the target trains away from the frozen draft (observed as accept rate: 0.00 followed by an out-of-range index assert in decode ~100 steps in).

Fused DSA indexer weights unmappable by ParameterMapper (deepseek_v2.py)
With use_dsa_indexer_fusion, the model has one bf16 indexer.wk_weights_proj per layer and no indexer.wk / indexer.weights_proj params. The checkpoint loader folds the unfused names via _load_fused_indexer_wk, but ParameterMapper — built from stacked_params_mapping and used by the p2p staging path — had no entry, so every indexer wk/weights_proj tensor sent by the trainer was silently dropped. Added the two stacked entries, gated on the fusion config (not a named_parameters() probe: the NextN subclass builds its mappings before its model exists), with dot-anchored patterns so already-fused checkpoint names cannot be rewritten into nonsense.

NextN name mapping for external weight mappers (deepseek_nextn.py)
mutate_weight_preload maps model.layers.{num_hidden_layers}.* to model.decoder.* so ParameterMapper.from_model on the draft model resolves converter-emitted nextn names the same way the loader does.

Registration was one-shot and swallowed failures (remote_instance_weight_transporter.py)
A transient connection failure while PUTting to the bootstrap server was logged and forgotten; the missing rank then surfaced minutes later as a 404 at the first weight update, far from the cause. Both registrations now retry (24 x 5s, timeout 10s) and raise after exhaustion.

Inverse ue8m0 scale transform for non-128-divisible row counts (fp8_utils.py)
The weights-checker comparator crashed on params like fused_qkv_a_proj_with_mqa (mn=2624): the inverse transform assumed the repeated-scale layout's row count is a multiple of the 128 block size. The main blocks and the partial tail block are now de-repeated separately.


CI States

Latest PR Test (Base): ❌ Run #32296566471
Latest PR Test (Extra): ❌ Run #32296566119
Latest PR Test (AMD ROCm 10): ➖ No AMD PR run found for this commit.

ishandhanani and others added 30 commits July 22, 2026 15:18
…32074)

Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
Signed-off-by: Connor Carpenter <connorc@nvidia.com>
Co-authored-by: Connor Carpenter <connorc@nvidia.com>
) (#32259)

Co-authored-by: Qiaolin Yu <liin1211@outlook.com>
…yer mtp draft extend cuda graph (#32254) (#32260)

Co-authored-by: Qiaolin Yu <liin1211@outlook.com>
…isoning spec verify capture (trtllm_mla + tc_piecewise) (#32288) (#32346)

Co-authored-by: Qiaolin Yu <liin1211@outlook.com>
…ense) (#18639)

Squashes the qwen_dense on-policy follow-up into the original FSDP2 commit:
the follow-up rewrote the norm_kwargs / rl_on_policy_target call sites this
commit introduced, so they cannot be applied independently.

Rebased onto v0.5.16: rl_on_policy_target predicates now go through the
sglang.srt.true_on_policy helpers, and every legacy get_global_server_args()
call site uses runtime_context.get_server_args() so v0.5.16's
test_legacy_global_ratchet baseline still holds.

Co-authored-by: maocheng23 <35615230+maocheng23@users.noreply.github.com>
…h/RL fixes (#25141, #29874, #31251)

Squashes the two follow-up PRs into the MoE-LoRA base commit: both rewrite the
mem_pool sharding helpers this commit introduces and cannot be applied
independently.

Rebased onto v0.5.16:
  - the shard probes keep v0.5.16's shared-MoE-over-full-TP carve-out at EP=1
    (`is_shared_moe_module`) and only fall through to the probed shard for
    non-MoE modules;
  - #29874's `free_lora` is dropped -- v0.5.16 already releases the slot on
    unload through `LoRAMemoryPool.remove_lora()`, which additionally zeroes the
    buffers for graph-captured replay, so `lora_manager.unload` needs no second
    call;
  - #31831's expected_checksums check is likewise already upstream in
    `tp_worker.load_lora_adapter_from_tensors`.

Co-authored-by: Ethan (Yusheng) Su <yushengsu@radixark.ai>
…psert (#27268, #31759, #30913)

Squashes the two follow-ups into the API commit: #31759 fixes this commit's
HTTP handler (the LoRAUpdateOutput was returned unserialized) and #30913
rewrites its registry-registration block to support in-place upsert, so
neither applies on its own.

Co-authored-by: Ethan (Yusheng) Su <yushengsu@radixark.ai>
Co-authored-by: Mathew Han <49226490+mathewjhan@users.noreply.github.com>
…22663)

Includes making Cohere2MoeConfig a dataclass so parameter mapping can
introspect config fields for P2P weight update.

Rebased onto v0.5.16, which extracted the remote-instance transfer engine out
of ModelRunner into RemoteInstanceWeightTransporter: the per-rank
RankParallelismConfig is now built in `init_engine()` and published from
`maybe_register_and_publish_weight_info()`, instead of the ModelRunner methods
this commit originally added. The hoisted deepseek expert_params_mapping also
picks up v0.5.16's broader `is_wint4afp8_or_wint4a16_config` predicate rather
than the old `quant_config.get_name() == "w4afp8"` check.

Co-authored-by: JensenFire <elevenji@outlook.com>
… for spec draft worker(s) (#27749, #28575, #18565, #22663, #28001, #29675, #27750)

Squash of the spec-draft distributed-update work and the weight-processing
session work: the latter rewrites iter_draft_runners()/get_model_runners() the
former introduces, so they cannot be applied independently.

  - Draft runners never join the update group, so the target receives the
    broadcast once and the weights are loaded into every selected runner.
  - Begin/EndWeightUpdate session chain: engine / http_server /
    tokenizer_control_mixin (pause-aware locking) -> scheduler ->
    SchedulerWeightUpdaterManager, with {target,draft,all} runner selectors.
  - loader.py: public post_load_weights plus restore_weight/postprocess_weight
    via _apply_quant_method_hook (skips LoRA wrappers).
  - weight checker: skip_tensor_list + role-prefixed overall_checksum over
    get_model_runners(selector), replacing _get_draft_model_runner.

Reimplemented onto v0.5.16, which had already extracted weight updating into
ModelRunner.weight_updater (WeightUpdater): the receive/load split lands there
as receive_weights_from_distributed() + load_weights() rather than on
ModelRunner, and the scheduler fan-out drives runner.weight_updater.*. The
worker-level update_weights_from_{distributed,tensor} entry points on
TpModelWorker / EAGLEWorkerV2 / NGRAMWorker are dropped: the scheduler now owns
the fan-out, and leaving them would be a second path that updates one runner
only. iter_runners() replaces the ad-hoc draft-runner discovery on every spec v2
worker (incl. DFlashWorkerV2). The unit test moves to test/registered/rl/, since
v0.5.16 retired test/srt/ from CI collection.

Co-authored-by: Yueming Yuan <yym022502@gmail.com>
Co-authored-by: JD-ETH <jaedon.guo@gmail.com>
Co-authored-by: maocheng23 <35615230+maocheng23@users.noreply.github.com>
…ht updates (#30421)

The per-rank attn_sink slice is built once and cached; an RL weight update
rewrites attn_sink in place, so decode keeps serving the pre-update sink.
Refresh the cache from post_load_weights instead.

Extended past the original PR for v0.5.16, which added deepseek_v4_dspark.py
with the same lazily-cached slice: refresh_attn_sink_cache moves from MQALayer
up to MqaAttentionBase (with an overridable _attn_sink_pad_width, since DSpark
pads q to _PAD_NUM_HEADS rather than padded_num_heads), and
DeepseekV4ForCausalLMDSpark gains the matching post_load_weights hook. Without
this, DSpark + RL would keep the same stale-sink bug this commit fixes for the
non-DSpark path.
… into a host-local checkpoint (#30366, #28524)

Folds in the two fixes #28524 made to this endpoint -- Annotated[..., Body()]
on the handler and keyword construction of PullWeightsReqOutput, which is a
kw_only msgspec Struct and would raise on the positional form. The rest of
#28524 (dumper / dump-comparator) is already upstream in v0.5.16 and is dropped.
…h the merged per-role payload

`_merge_checksum_payloads` folds each runner's payload into one, tagging every
entry with its role, so `parallelism_info` is a LIST of per-role entries. That
shape was never validated on v0.5.15 -- v0.5.16 added the msgspec wire structs
plus `msgspec.convert(p, ChecksumInfo)` in `check_weights`, and the wire struct
still declared a single `ParallelismInfo` with no `role`, so every
check_weights('checksum') call would have raised on conversion.

This is the shape miles already consumes: `checksum_utils._gpu_rank` iterates
`parallelism_info` and reads `role_info["rank"]` per role, and its tests cover
the target+draft case sharing one GPU rank.
…t to v0.5.16's parallel state

Two follow-ups to [14/24]'s P2P weight-update support, both caused by v0.5.16
retiring the module-level dp-attention accessors. Found by CI: every GPU stage
that initialises the remote-instance transfer engine died at startup with

    remote_instance_weight_transporter.py:56 init_engine
      -> RankParallelismConfig.from_parallel_state
    ImportError: cannot import name 'get_attention_cp_rank'
                 from 'sglang.srt.layers.dp_attention'

1. `from_parallel_state` imported get_attention_{tp,dp,cp}_{rank,size} from
   dp_attention. v0.5.16 removed all six; the attention-side ranks and sizes now
   live on the runtime ParallelState, so read them from `get_parallel()`.

2. `ParallelismContext` set `dp_attention._ENABLE_DP_ATTENTION_FLAG`, which
   v0.5.16 replaced with `get_flags().dp.enabled`. That assignment would not have
   raised -- it would have created a stale attribute nobody reads, leaving
   `is_dp_attention_enabled()` reporting the ambient value instead of the
   context's and silently building a replica with the wrong sharding. The flag is
   now saved, set and restored on the runtime flags instead, and dropped from
   _DA_GLOBALS.
Three stale references left by the rebase. The cherry-picks applied cleanly
because the surrounding lines matched, so nothing surfaced until the code ran.

1. model_runner.forward: `decode_cuda_graph_runner.num_tokens_per_bs` ->
   `.captured_req_width`. v0.5.16 renamed the per-request capture width (and
   `ModelRunner.decode_num_tokens_per_bs` -> `decode_num_tokens_per_req`), so
   every decode with a captured graph raised AttributeError.
   From [2/25], R3 DeepEP/MTP support.

2. lora/backend/base_backend: `dp_attention.get_attention_tp_size()` ->
   `get_parallel().attn_tp_size`. Same removal as [24/25]; this one is a lazy
   import so it only fired on the MoE-LoRA DP-attention path.

3. lora/backend/base_backend: `layers.utils.cp_utils` ->
   `layers.cp.padding` for get_cp_padding_align_size. The module moved in
   v0.5.16; every other caller in the tree already uses the new path.

Found 1 and 2 in CI; found 3 by then resolving every sglang symbol imported by
lines this branch adds against the v0.5.16 tree, which is how it should have been
checked in the first place.
@Jiminator Jiminator closed this Sep 14, 2026
@Jiminator
Jiminator deleted the yueming/p2p-registry-fixes branch September 14, 2026 04:44
@Jiminator
Jiminator deleted the branch sglang-miles September 14, 2026 04:44
@alexnails
alexnails restored the yueming/p2p-registry-fixes branch September 14, 2026 05:46
@hnyls2002 hnyls2002 reopened this Sep 14, 2026
@github-actions github-actions Bot added quant LLM Quantization dependencies Pull requests that update a dependency file lora Multi-modal multi-modal language model blackwell SM100/SM120 model-gateway jit-kernel labels Sep 14, 2026
@Rockdu
Rockdu force-pushed the sglang-miles branch 2 times, most recently from 865cb64 to 29b34bc Compare October 31, 2026 06:43

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blackwell SM100/SM120 deepseek dependencies Pull requests that update a dependency file jit-kernel lora model-gateway Multi-modal multi-modal language model quant LLM Quantization

Projects

None yet

Development

Successfully merging this pull request may close these issues.