Repository navigation
[PCP] Fewer V4 prefill gathers and fix PCP+TBO accuracy #2789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 12 commits
16e4f7b
dc90ce2
7017852
222feeb
b491763
6b49222
9d1db12
e182354
e8ff821
7e2920f
fd0893d
319ffe1
fd912b7
c889ff9
65fed29
c9b9de7
aee343c
1e582ae
5e4e8bb
c5b86da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,8 @@ | |
| get_prefill_context_model_parallel_world_size, | ||
| ) | ||
|
|
||
| from atom.utils import upload_numpy | ||
|
|
||
|
|
||
| class PcpBalGroup(NamedTuple): | ||
| """One request group for PCP+TBO request-boundary split prefill. | ||
|
|
@@ -105,7 +107,12 @@ def pcp_round_robin_split( | |
| # Divisibility by pcp_size is guaranteed upstream by pcp_pad_len (callers | ||
| # pad before splitting); the view below would error if violated. | ||
| rest = tuple(input_.shape[1:]) | ||
| return input_.view(-1, pcp_size, *rest)[:, pcp_rank].contiguous() | ||
| # Not `.contiguous()`: a one-row shard counts as contiguous while its row | ||
| # stride is still pcp_size, which kernels that check strides reject (aiter's | ||
| # cached-positions RoPE). | ||
| return input_.view(-1, pcp_size, *rest)[:, pcp_rank].clone( | ||
| memory_format=torch.contiguous_format | ||
| ) | ||
|
|
||
|
|
||
| def pcp_allgather_rerange( | ||
|
|
@@ -127,16 +134,47 @@ def pcp_allgather_rerange( | |
| group = get_pcp_group() | ||
| # aiter all_gather(dim=0) returns rank-major concat: [pcp*L, *rest]. | ||
| gathered = group.all_gather(input_.contiguous(), dim=0) | ||
| local_len = input_.shape[0] | ||
| rest = tuple(input_.shape[1:]) | ||
| return _rerange(gathered, pcp_size, input_.shape[0]) | ||
|
|
||
|
|
||
| def gather_rows(gather, rows): | ||
| """`gather(list of tensors) -> list of tensors` over `rows`, one-byte | ||
| dtypes carried as uint8: aiter's grouped `all_gatherv` copies bytes | ||
| whatever the dtype, but its PyNCCL fallback has no fp8 / e8m0 / fp4x2.""" | ||
| gathered = gather( | ||
| [t.view(torch.uint8) if t.element_size() == 1 else t for t in rows] | ||
| ) | ||
| return tuple(g.view(t.dtype) for g, t in zip(gathered, rows)) | ||
|
|
||
|
|
||
| def pcp_allgather_rerange_many( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fixing it here and leaving those (Fix-then-sweep) keeps both paths alive.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| inputs: list[torch.Tensor], pcp_size: int | None = None | ||
| ) -> list[torch.Tensor]: | ||
| """`pcp_allgather_rerange` of every tensor in `inputs` (same row count) in | ||
| one grouped collective.""" | ||
| if pcp_size is None: | ||
| pcp_size = get_pcp_world_size() | ||
| if pcp_size <= 1: | ||
| return list(inputs) | ||
| assert all(t.shape[0] == inputs[0].shape[0] for t in inputs), [ | ||
| t.shape[0] for t in inputs | ||
| ] | ||
| comm = get_pcp_group().device_communicator | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aiter has no kernel for pcp 6 yet, and we pin no aiter version. keep the per-tensor fallback until #7149 lands?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional. |
||
| gathered = gather_rows( | ||
| lambda ts: comm.all_gatherv(ts, dim=0), [t.contiguous() for t in inputs] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pcp 6 will die inside aiter without #7149, and nothing checks the size at startup. validate at engine init?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional. |
||
| ) | ||
| return [_rerange(g, pcp_size, t.shape[0]) for g, t in zip(gathered, inputs)] | ||
|
|
||
|
|
||
| def _rerange(gathered: torch.Tensor, pcp_size: int, local_len: int) -> torch.Tensor: | ||
| # rank-major [pcp, L, *rest] -> transpose -> token-major [L, pcp, *rest] | ||
| # -> flatten to [L*pcp, *rest] == original global order. | ||
| out = ( | ||
| rest = tuple(gathered.shape[1:]) | ||
| return ( | ||
| gathered.view(pcp_size, local_len, *rest) | ||
| .transpose(0, 1) | ||
| .reshape(pcp_size * local_len, *rest) | ||
| ) | ||
| return out | ||
|
|
||
|
|
||
| # ==== MoE-path PCP collectives (rank-major gather + reduce_scatter) ==== | ||
|
|
@@ -246,10 +284,22 @@ def pcp_pad_dense(t: torch.Tensor, n_pad: int) -> torch.Tensor: | |
| return torch.cat([t, t.new_zeros(n_pad, *t.shape[1:])], dim=0) | ||
|
|
||
|
|
||
| def pcp_local_indptr(indptr: np.ndarray, owned: np.ndarray) -> np.ndarray: | ||
| """Host side of `pcp_reindex_ragged`: the prefix sum of the `owned` | ||
| queries' segment lengths in `indptr`. Owned ids past its last query are | ||
| padding queries, whose segments are empty.""" | ||
| lens = np.diff(indptr) | ||
| lens = np.pad(lens, (0, max(int(owned.max(initial=-1)) + 1 - lens.shape[0], 0))) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nothing checks the host indptr matches the device one. add a cheap shape/last-value assert so a mismatch fails loud instead of giving wrong indices?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in fd912b7: a length check at both reindex entries (rank and PCP+TBO group). It reads shapes only; comparing values would need a device read-back, which is the sync this PR removes. |
||
| out = np.zeros(owned.shape[0] + 1, dtype=np.int32) | ||
| np.cumsum(lens[owned], out=out[1:]) | ||
| return out | ||
|
|
||
|
|
||
| def pcp_reindex_ragged( | ||
| kv_indptr: torch.Tensor, # [T_global + 1] int32 — global per-query prefix sum | ||
| kv_indices: torch.Tensor, # [kv_indptr[-1]] — ragged packed values | ||
| owned_q: torch.Tensor, # [T_local] long — global query ids this rank owns | ||
| indptr_local_cpu: np.ndarray, # [T_local + 1] — `pcp_local_indptr` | ||
| ) -> tuple[torch.Tensor, torch.Tensor]: | ||
| """Reindex a ragged (indptr, indices) pair down to this rank's queries. | ||
|
|
||
|
|
@@ -262,17 +312,15 @@ def pcp_reindex_ragged( | |
| Used to shard the per-query prefill index buffers (kv_indptr/kv_indices | ||
| _prefix_swa / _extend) to 1/W while the values themselves still point into | ||
| the full KV (paged unified_kv) / full extend kv tensor. | ||
|
|
||
| `indptr_local` comes from the host (`indptr_local_cpu`), so sizing the | ||
| output reads nothing back from the device. | ||
| """ | ||
| device = kv_indptr.device | ||
| owned_q = owned_q.to(device) | ||
| starts = kv_indptr[owned_q] # [T_local] | ||
| ends = kv_indptr[owned_q + 1] # [T_local] | ||
| lens = ends - starts # [T_local] per-owned-query segment length | ||
| indptr_local = torch.zeros( | ||
| owned_q.shape[0] + 1, dtype=kv_indptr.dtype, device=device | ||
| ) | ||
| torch.cumsum(lens, dim=0, out=indptr_local[1:]) | ||
| total = int(indptr_local[-1].item()) | ||
| indptr_local = upload_numpy(indptr_local_cpu, device) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performance item 1:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The per-step reindex kernel and its asserts are removed (see the indptr comment above), and so is the |
||
| total = int(indptr_local_cpu[-1]) | ||
| if total == 0: | ||
| return indptr_local, kv_indices.new_empty(0) | ||
| # Build a gather map: for each output slot, which source index to read. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,7 @@ | |
| get_pcp_rank, | ||
| get_pcp_world_size, | ||
| pcp_is_enabled, | ||
| pcp_local_indptr, | ||
| pcp_owned_rows_by_request, | ||
| pcp_pad_dense, | ||
| pcp_pad_indptr, | ||
|
|
@@ -388,6 +389,9 @@ class AttentionMetaData_DSV4(AttentionMetaData): | |
| (same `kv` shared by all 3 ratios; one builder pass).""" | ||
| kv_indptr_extend: torch.Tensor | None = None | ||
| """[total_tokens + 1] int32 GPU — packed cumsum of `extend_count`.""" | ||
| paged_prefill_indptr_cpu: dict[str, np.ndarray] | None = None | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We went further than mirroring: each rank now stages its own indptrs from numpy through the CpuGpuBuffer publication group and launches the paged-prefill index kernel only over the tokens it owns, so the host mirror, the pad/reindex kernels and |
||
| """Host copies of the four `kv_indptr_*` above, keyed by field name. PCP | ||
| reindexes from these, so it never reads the GPU copies back.""" | ||
|
|
||
|
|
||
| class DeepseekV4Backend(AttentionBackend): | ||
|
|
@@ -2868,10 +2872,13 @@ def _apply_pcp_reindex( | |
| # on its own pcp pad). Either way the divisor is pcp_size. | ||
| padded_total = pcp_pad_len(total_tokens, pcp_size) | ||
| n_pad = padded_total - total_tokens | ||
| # This rank's round-robin queries, generated on the GPU. | ||
| # This rank's round-robin queries, on the GPU and on the host. | ||
| owned_q = torch.arange(get_pcp_rank(), padded_total, pcp_size, device=device) | ||
| owned_cpu = np.arange(get_pcp_rank(), padded_total, pcp_size) | ||
|
|
||
| # --- ragged per-query buffers: pad indptr to padded_total, then 1/W --- | ||
| indptr_cpu = attn_metadata.paged_prefill_indptr_cpu | ||
|
yitingw1 marked this conversation as resolved.
Resolved
|
||
| local_indptr_cpu = {} | ||
| for ind_attr, idx_attr in ( | ||
| ("kv_indptr_prefix_swa", "kv_indices_prefix_swa"), | ||
| ("kv_indptr_prefix_csa", "kv_indices_prefix_csa"), | ||
|
|
@@ -2883,9 +2890,14 @@ def _apply_pcp_reindex( | |
| if indptr is None or indices is None: | ||
| continue | ||
| indptr = pcp_pad_indptr(indptr, n_pad) # dummy queries: 0-length KV | ||
| new_indptr, new_indices = pcp_reindex_ragged(indptr, indices, owned_q) | ||
| local_cpu = pcp_local_indptr(indptr_cpu[ind_attr], owned_cpu) | ||
| new_indptr, new_indices = pcp_reindex_ragged( | ||
| indptr, indices, owned_q, local_cpu | ||
| ) | ||
| setattr(attn_metadata, ind_attr, new_indptr) | ||
| setattr(attn_metadata, idx_attr, new_indices) | ||
| local_indptr_cpu[ind_attr] = local_cpu | ||
| attn_metadata.paged_prefill_indptr_cpu = local_indptr_cpu | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. overwrites the full-batch indptr in place, a second call would silently give too-short kv. keep it local instead?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each metadata object is reindexed once: the non-TBO path reindexes the full batch once, and the PCP+TBO path reindexes each group's fresh There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this field is documented as the global host copies, now we write local ids into it.. use a separate field?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworded in fd912b7: the host copies mirror whichever |
||
|
|
||
| # --- dense per-token fields: pad then round-robin-slice to 1/W --- | ||
| if attn_metadata.skip_prefix_len_csa is not None: | ||
|
|
@@ -3184,6 +3196,8 @@ def _build_ubatch_prefill_metadata_balanced( | |
|
|
||
| # ---- per-token DSV4 fields sliced by the GLOBAL token range [gts,gte) ---- | ||
| owned = torch.arange(gts, gte, device=device) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performance item 2: a group's queries are the contiguous global range
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of slicing the whole-batch meta, each TBO group now stages its own |
||
| owned_cpu = np.arange(gts, gte) | ||
| ub.paged_prefill_indptr_cpu = {} | ||
| for ind_attr, idx_attr in ( | ||
| ("kv_indptr_prefix_swa", "kv_indices_prefix_swa"), | ||
| ("kv_indptr_prefix_csa", "kv_indices_prefix_csa"), | ||
|
|
@@ -3194,7 +3208,11 @@ def _build_ubatch_prefill_metadata_balanced( | |
| indices = getattr(src, idx_attr, None) | ||
| if indptr is None or indices is None: | ||
| continue | ||
| ni, nx = pcp_reindex_ragged(indptr, indices, owned) | ||
| local_cpu = pcp_local_indptr( | ||
| src.paged_prefill_indptr_cpu[ind_attr], owned_cpu | ||
| ) | ||
| ni, nx = pcp_reindex_ragged(indptr, indices, owned, local_cpu) | ||
| ub.paged_prefill_indptr_cpu[ind_attr] = local_cpu | ||
| # kv_indices_extend are ROW offsets into the per-fwd kv_full tensor. | ||
| # In the full metadata they index the WHOLE sequence's kv_full [0,T); | ||
| # for this group kv_full only holds the group's tokens (global order | ||
|
|
@@ -3856,6 +3874,12 @@ def _build_paged_prefill_meta( | |
| attn_metadata.kv_indptr_prefix_csa = csa_indptr | ||
| attn_metadata.kv_indices_prefix_hca = hca_indices[:hca_total] | ||
| attn_metadata.kv_indptr_prefix_hca = hca_indptr | ||
| attn_metadata.paged_prefill_indptr_cpu = { | ||
| "kv_indptr_extend": ext_indptr_np, | ||
| "kv_indptr_prefix_swa": swa_indptr_np, | ||
| "kv_indptr_prefix_csa": csa_indptr_np, | ||
| "kv_indptr_prefix_hca": hca_indptr_np, | ||
| } | ||
| attn_metadata.skip_prefix_len_csa = skip_csa_gpu | ||
| attn_metadata.envelope_rows = envelope_rows | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gather_rowsnow lives inpcp_utils, but its callers include DP (dp_gather_routed_rows) and the TP/DPcomm_fused_moe, socomm_fused_moeimports a general gather from the PCP module. It belongs in a general distributed comm module (next totoken_transportor similar), with PCP importing it too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to
atom/distributed/comm_utils.pyas a transport-agnostic helper. It now only reinterprets 1-byte dtypes as uint8 and passes everything else through unchanged; the float carriers are gone.