Overlap moe comms with collective matmul#4295
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| use_custom_sort_vjp: true # whether to use a custom VJP sort for efficient backward pass processing in sparse matmul | ||
| use_ring_of_experts: false # whether to use ring of experts for sparse matmul expert parallelism | ||
| chunk_ag_gmm: false # whether to chunk All-Gather and GMM computation to overlap communication | ||
| num_chunks: 0 # number of chunks for overlapping All-Gather and GMM computation along embedding dimension |
There was a problem hiding this comment.
I suggest giving them more specific names to distinguish with the other chunking feature in cl/938756606
| # TODO(tgale, enriqueps, apaske): Fuse this transposition into the tgmm. | ||
| drhs = drhs.swapaxes(1, 2) if transpose_rhs else drhs | ||
| return dlhs, drhs, None, None, grad | ||
| return dlhs, drhs, None, None, grad, grad if partial_sum_fwd is not None else None |
There was a problem hiding this comment.
maybe move the if-else logic out of the return for better readability
| assert partial_sum.shape == (size_m, size_n) | ||
| assert partial_sum.shape[-1] == size_n | ||
| # lhs's m dimension can sometimes be padded to wi_tile_fwd_batch_seq | ||
| assert partial_sum.shape[0] <= size_m |
There was a problem hiding this comment.
for my understanding is this change specific due to chunking feature or a general fix?
There was a problem hiding this comment.
This occurs when embedding dim is not a multiple of wi_tile_fwd_batch_seq, so m in padded but partial sum is not. This is specifically for chunking because partial sum is only used for chunking feature
| global_sorted_experts=selected_experts, | ||
| use_custom_sort_vjp=self.config.use_custom_sort_vjp, | ||
| use_ragged_sort=self.config.use_ragged_sort, | ||
| ) |
There was a problem hiding this comment.
we could move these logics to a new function named ra2a_and_route, mirroring your changes earlier.
| and callable(getattr(rngs, "params")) | ||
| else [rngs] * self.config.num_chunks | ||
| ) | ||
| # TODO: is this valid? |
There was a problem hiding this comment.
add a correctness test protecting this feature for convergence if not?
| weight_gather, | ||
| partial_accum0=ps0, | ||
| partial_accum1=ps1, | ||
| ) |
There was a problem hiding this comment.
I think it might be a good idea to move all the logic in if-branch to a separate function, called moe_emb_chunking or better names.
| (last_x_chunk, ps0, ps1), _ = jax.lax.scan( | ||
| scan_fn, | ||
| (cur_x_chunk, partial_sum0, partial_sum1), | ||
| jnp.arange(self.config.num_chunks - 1), |
There was a problem hiding this comment.
I think you could use length=self.config.num_chunks-1 instead of arange. Also, have you tried performance when the scan is unrolled? e.g. add unroll=self.config.num_chunks-1, curious the performance diff.
There was a problem hiding this comment.
That's a good idea! I'll give it a try. I'm still working on improving the performance of this feature and this PR is just to implement the feature. I can experiment & make changes if needed in a follow up PR
| mesh = Mesh(devices_array, cfg.mesh_axes) | ||
| variables, expected_output = self.get_expected_output(rng_model, hidden_states, cfg, mesh) | ||
| actual_output, _, _ = self.get_moe_output(variables, hidden_states, cfg, mesh) | ||
| self.assertTrue(jax.numpy.allclose(expected_output, actual_output, rtol=1e-02, atol=1e-02, equal_nan=False)) |
There was a problem hiding this comment.
oh I see the correctness test, nice!
|
|
||
| assert_arrays_all_close(actual, expected) | ||
|
|
||
| @pytest.mark.skip(reason="Test takes too long, can run locally to verify changes b/528087469") |
There was a problem hiding this comment.
emmm I honestly think these tests gonna never get run....
| self.assertEqual(actual.shape, (num_local_groups, in_size, out_size)) | ||
| assert_arrays_all_close(actual, expected) | ||
|
|
||
| @pytest.mark.skip(reason="Test takes too long, can run locally to verify changes b/528087469") |
There was a problem hiding this comment.
emmm I honestly think these tests gonna never get run.... can we make it simple and fast?
There was a problem hiding this comment.
We are going to use tokamax kernels directly in the future instead of forking it in maxtext, so this is only going to be here temporarily
NuojCheng
left a comment
There was a problem hiding this comment.
Overall LGTM. Leave some comments, thank you Shuwen and great work!
| Validates that use_moe_emb_chunking is used with supported settings. | ||
| """ | ||
| if self.use_moe_emb_chunking: | ||
| logger.warning( |
There was a problem hiding this comment.
I would prefer to not have this warning, but up to you
| use_custom_sort_vjp: true # whether to use a custom VJP sort for efficient backward pass processing in sparse matmul | ||
| use_ring_of_experts: false # whether to use ring of experts for sparse matmul expert parallelism | ||
| use_moe_emb_chunking: false # whether to chunk All-Gather and GMM computation to overlap communication | ||
| num_moe_emb_chunks: 0 # number of chunks for overlapping All-Gather and GMM computation along embedding dimension |
There was a problem hiding this comment.
I would only have oneuflag - num_moe_emb_chunks - when its set to 0 its equivalent to false
There was a problem hiding this comment.
good idea, updated
There was a problem hiding this comment.
I would specify "overlapping all-gather" with "overlapping token all-gather" to distinguish it from FSDP all gathers
| """Performs both across device and within device token routing/sorting""" | ||
| num_ep = self.get_expert_parallelism_size() | ||
| expert_shard_id = jax.lax.axis_index(self._expert_parallelism_name) if num_ep > 1 else 0 | ||
| def roe_ag_and_route(x, logits, pre_bias_logits, num_ep, expert_shard_id, rngs, input_ids=None): |
| ), | ||
| ) | ||
|
|
||
| def route(x, logits, pre_bias_logits, rngs, input_ids=None): |
|
|
||
| def moe_emb_chunking( | ||
| x, logits, pre_bias_logits, w0, w1, w0_bias, w1_bias, wo_bias, sharded_input_ids, rngs, embed_dim | ||
| ): |
There was a problem hiding this comment.
can you include a comment for this method?
Implements Overlapped All-Gather and GMM Computation (Collective Matmul) for Ring of Experts (RoE) in MoE layers to hide communication overhead and token sorting latency. The changes include: - Chunks input activations along the contracting embedding dimension (embed_dim // num_moe_emb_chunks). While chunk i computes GMM up-projections, chunk i+1 concurrently executes Ring of Experts All-Gather and routing across expert parallel shards. - Update kernel to use in-place Partial Sum Accumulation to avoid unnecessary copies using input_output_aliases. Testing - Layer Tests (tests/unit/moe_test.py): Verified chunked GMM v2 routing, partial sum accumulation, and MLP bias addition (test_moe_emb_chunking_gmm_v2 with mlp_bias=True). - AOT Compilation Tests (tests/unit/train_compile_test.py): Verified ahead-of-time HLO compilation and sharding on multi-slice topologies for chunked MoE (test_moe_emb_chunking and test_moe_emb_chunking_with_mlp_bias). - Low-Level Pallas Kernel Tests (tests/unit/pallas_mosaic_tpu_v2_kernel_test.py): Verified numerical correctness and in-place buffer aliasing for partial sum accumulation across GMM (test_gmm_partial_sum0/1) and TGMM with zero-initialized empty expert groups (test_tgmm_empty_group_with_partial_sum0-3). - End-to-End Execution & Profiling: Verified E2E training execution on v6e-8 TPU VM (deepseek3-tiny with num_moe_emb_chunks=4) and generated XPlane / XProf traces confirming communication-compute overlap.
Description
Implements Overlapped All-Gather and GMM Computation (Collective Matmul) for Ring of Experts (RoE) in MoE layers to hide communication overhead and token sorting latency. The changes include:
embed_dim // num_chunks). While chunkicomputes GMM up-projections, chunki+1concurrently executes Ring of Experts All-Gather and routing across expert parallel shards.input_output_aliasesTesting
tests/unit/moe_test.py): Verified chunked GMM v2 routing, partial sum accumulation, and MLP bias addition (test_chunking_gmm_v2withmlp_bias=True).tests/unit/train_compile_test.py): Verified ahead-of-time HLO compilation and sharding on multi-slice topologies for chunked MoE (test_moe_chunkingandtest_moe_chunking_with_mlp_bias).tests/unit/pallas_mosaic_tpu_v2_kernel_test.py): Verified numerical correctness and in-place buffer aliasing for partial sum accumulation across GMM (test_gmm_partial_sum0/1) and TGMM with zero-initialized empty expert groups (test_tgmm_empty_group_with_partial_sum0-3).deepseek3-tinywithchunk_ag_gmm=True,num_chunks=4) and generated XPlane / XProf traces confirming communication-compute overlap.Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.