Skip to content

fix(p2p): publish parameter dtypes for weight transfers - #43715

Closed
Laz4rz wants to merge 1 commit into
sgl-project:yueming/p2p-s1from
Laz4rz:upstream/p2p-dtype-guard
Closed

Laz4rz wants to merge 1 commit into
sgl-project:yueming/p2p-s1from
Laz4rz:upstream/p2p-dtype-guard

Conversation

@Laz4rz

@Laz4rz Laz4rz commented Oct 11, 2026 •

Copy link
Copy Markdown

Motivation

P2P senders need the receiver's actual parameter dtype before writing directly into its memory. The current address and size metadata cannot distinguish BF16 from FP16: both use two bytes per element, but interpret those bytes differently.

Built on #42820. Companion PR: radixark/miles#4016 — validate receiver dtypes before P2P weight transfers. The companion extends radixark/miles#3969 and rejects incompatible transfers using this metadata. This SGLang change publishes the information; it does not itself enforce the check.

Modifications

  • Publish weight_dtypes from the registered model's named parameters, using names such as bfloat16.
  • Store and return the mapping with the existing (role, tp_rank) metadata, preserving separate target and draft records.
  • Keep the existing remote_instance_transfer_engine_info tuple unchanged. Publishers without dtype metadata return an empty mapping, and consumers that read only the original field continue working.
  • Extend the role-publication unit test to check the dtype mappings and preservation of the existing response.

Deploy this change before the stricter companion Miles sender, or deploy both together.

Accuracy Tests

Tested the paired changes in a fork on Qwen/Qwen2.5-0.5B-Instruct, with two runs per case below. Each used two H100 nodes, one trainer GPU and one inference GPU. All six runs completed 11 training steps with initial weight equality, successful EFA transfers and finite nonzero gradients.

These are short execution checks with synthetic rewards, not convergence or reconnect tests. Every run logged an existing forced-termination warning during inference shutdown.

The repository-pinned pre-commit hooks passed locally on all 3 PR files without modifying them, and the upstream full-repository lint job passed. The extended runtime unit test has not been rerun on this upstream port; the fork benchmark results do not validate this exact tree.

Speed Tests and Profiling

Case First weight sync Warm-sync median Registered memory per sender
Previous P2P, with sender dtype corrected 13.73–13.95 s 0.275–0.276 s 0.92 GiB
Loader-based P2P, without this guard 10.47–10.55 s 0.280–0.282 s 1.00 GiB
Loader-based P2P + dtype guard 10.62–10.77 s 0.269–0.275 s 1.00 GiB

Ranges show the two runs; each warm median covers ten syncs. Timings include the whole weight-sync operation and GPU completion. Runs used different node pairs in parallel, so small differences should not be treated as speedups. Memory counts application buffer registrations, not total process memory; the loader-based implementation uses two 512 MiB buffers.

The companion Miles guard checked 170 BF16 tensor layouts once per connection. Local metadata validation averaged 94 µs with the guard versus 51 µs without, an observed difference of approximately 43 µs per connection, not per training step. Publisher serialization and HTTP metadata overhead were not isolated, and no speed benchmark was run on this upstream port.

Checklist

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.

2 participants