Repository navigation
Conversation
Port the native-parameter MOK integration and alias metadata preservation from dev onto current main. Forward packed routing metadata and use main's precision-aware recompute entry point. Invalidate derived weight views on parameter conversion and retain the existing optional dependency pin. Based on dev PRs NVIDIA#6572 and NVIDIA#7289 and the earlier main adaptation NVIDIA#7695. Co-authored-by: qizhang <qizhang@nvidia.com> Co-authored-by: Xin Yao <xiny@nvidia.com> Co-authored-by: HaochenYuan <haocheny@nvidia.com> Co-authored-by: Ajay Balasa <abalasa@nvidia.com> Signed-off-by: Deyu Fu <deyuf@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test 1cdac4a |
|
Does this PR includes #7391 content? |
| "MOK does not support overlap_dispatch_backward_with_experts_wgrad; " | ||
| "the megakernel never runs the native dispatch path" | ||
| ) | ||
| if self.tensor_model_parallel_size != 1 or self.expert_tensor_parallel_size != 1: |
There was a problem hiding this comment.
Does it mean that we cannot do TP2 to dense modules and shared expert while still doing ETP=1 for experts?
If I remember correctly, MOK fuses shared expert in it, but it should be possible to disable shared expert compute and use Mcore's shared expert implementation so that we can do the above operation.
There was a problem hiding this comment.
Yes, TP2/ETP1 is currently rejected. The pinned MoK API requires shared-expert weights and has no routed-only switch. A native shared-expert fallback makes sense as a follow-up, with TP/SP routing and backward validation before relaxing this guard.
| raise ValueError("MOK requires a shared expert") | ||
| if self.moe_shared_expert_gate or self.moe_shared_expert_overlap: | ||
| raise ValueError("MOK does not support the MCore shared-expert gate/overlap") | ||
| if self.moe_latent_size is not None: |
There was a problem hiding this comment.
This should be possible to support since latent moe just has two projections, one before dispatch and another one after combine?
There was a problem hiding this comment.
In MCore those projections wrap only the routed branch; the shared expert consumes the original hidden states. Wrapping the fused MOK call would also project its shared branch and change that behavior. Native latent MoE therefore needs the routed-only/native-shared path first. I kept this guard and added the reason.
| raise ValueError("MOK does not support the MCore shared-expert gate/overlap") | ||
| if self.moe_latent_size is not None: | ||
| raise ValueError("MOK does not support latent MoE") | ||
| if self.recompute_modules and "shared_experts" in self.recompute_modules: |
There was a problem hiding this comment.
Like we discussed above, not sure if we have an application where it's reasonable to unfuse shared expert so that we can apply TP or maybe apply mxfp8 to it, since MOK's shared expert is in bf16?
There was a problem hiding this comment.
Yes, a separate MCore shared expert could provide TP and MXFP8 shared compute, as well as shared-expert-only recompute. That is a useful follow-up to benchmark, but it needs a routed-only MOK interface. The pinned API requires fused shared weights, so this port keeps the current path.
|
|
||
| if self.moe_megakernel_backend == "mok": | ||
| if self.cuda_graph_impl in ("local", "transformer_engine"): | ||
| if not self.cuda_graph_modules: |
There was a problem hiding this comment.
this if not self.cuda_graph_modules means full cuda graph?
If that's the case we should be able to actually do full cg if MOK can be integrated with paged stashing?
On the other hand, the amount of tokens before dispatch is static, if we are able to redo the dispatch in backward pass and overlap it with compute, it's also possible to capture cuda graph of MOK even without paged stashing?
There was a problem hiding this comment.
Here an empty list means whole-layer capture for local/transformer_engine; full_iteration is already allowed by the config. Per-layer replay needs the saved MOK schedule/activations to survive pending microbatches. Paged stashing or redispatch/recompute could provide that, but the current bridge implements neither. I clarified this distinction and kept the per-layer guard; GPU replay qualification remains pending.
|
@zhongbozhu #7391 is not included in this port. It adds the Qwen3.5 shared-output gate, which requires a newer MoK API than main's pinned |
| "megatron/core/transformer/moe/README.md" | ||
| ) from exc | ||
|
|
||
| if not config.gradient_accumulation_fusion: |
There was a problem hiding this comment.
What's the precision of wgrad accumulation? Assume we have D = A * B + C, does the accumulation precision depend on the dtype of C?
That said, if we keep C dtype to bf16, will the addition precision also be bf16?
There was a problem hiding this comment.
The GEMM accumulator is FP32 in both cases. With FP32 main_grad, it stays FP32 through the reduction-store. With BF16 main_grad, each contribution is rounded to BF16 before the BF16 reduction-store, so persistent accumulation is BF16. The shared weight dtype does not select this; main_grad.dtype does. The pinned API also requires shared/routed main-grad buffers to have the same dtype. Documented this from the kernel epilogue.
| def quantized_routed_weights(self): | ||
| """Build or refresh cached MOK routed-weight views.""" | ||
| needs_weight_view_update = self._routed_weight_view_cache is None or ( | ||
| self.use_mxfp8_weights and self.is_first_microbatch |
There was a problem hiding this comment.
Why do we need to update the view under use_mxfp8_weights, which is essentially under fp8 param gather?
There was a problem hiding this comment.
The cache includes converted MXFP8 scale buffers. Payloads still alias TE storage; after an optimizer update/parameter sync, the scale values change, so their MOK layout must be refreshed on the first microbatch. This happens in place to preserve descriptor addresses, and later microbatches reuse it. It does not requantize or copy the FP8 weight payload. Added this explanation in the method.
| self.topk = config.moe_router_topk | ||
| self.swiglu_limit = config.activation_func_clamp_value | ||
| self.use_mxfp8_weights = bool( | ||
| config.fp8 is not None and config.fp8_recipe == "mxfp8" and config.fp8_param |
There was a problem hiding this comment.
what about reuse_grad_buf_for_mxfp8_param_ag?
There was a problem hiding this comment.
That path gathers into shared staging storage, then copy_tensors_to_quantized_params requantizes the native TE parameters before the staging buffer is cleared. MOK aliases the TE payloads. I removed this guard in 951fd137d5: post_all_gather_processing is a no-op for MXFP8 in the pinned TE, so its presence is not the right capability check. Native rowwise/columnwise payload and scale checks remain. Full distributed validation of this combination is still pending.
Signed-off-by: Deyu Fu <deyuf@nvidia.com>
|
/ok to test 951fd13 |
| return flat.view(shape) | ||
|
|
||
|
|
||
| def _swizzle_mxfp8_scale( |
There was a problem hiding this comment.
we can fuse swizzle into quantization kernel by TE quantizer setting optimize_for_gemm?
There was a problem hiding this comment.
Yes, for compute-only MXFP8 tensors; TE's swizzled layout matches MOK. Here we alias native FP8 parameters, and the pinned TE explicitly disables preswizzling for primary FP8 weights. Dequantization and parameter-update paths still expect compact scales, so flipping this flag alone is not safe. I'll keep the zero-copy payload and cached scale conversion in this port; removing that conversion needs support across the native parameter lifecycle.
| f"expert={expert}, got={tuple(column_scale.shape)}, " | ||
| f"expected={(rows // 32, columns)}" | ||
| ) | ||
| _swizzle_mxfp8_scale(row_scale.unsqueeze(0), rows=rows, columns=columns, out=row_out) |
There was a problem hiding this comment.
same comment^, maybe we can leverage optimize_for_gemm in TE quantizer to fuse this swizzle
There was a problem hiding this comment.
Same primary-parameter limitation here. This refresh only updates the cached MOK scale bytes on the first microbatch after parameter sync; later microbatches reuse them. Keeping it for this port, with quantize/swizzle fusion as a follow-up once the native parameter lifecycle supports that layout.
What does this PR do?
Enable
moe_megakernel_backend=mokon main. MCore supplies routing and owns the expert parameters, optimizer state, and checkpoints; MoK executes dispatch, routed/shared experts, and combine.Ports #6572 and #7289, building on the earlier main adaptation in #7695. Related to #5795 and #7756. Uses the MoK revision already installed by #7696.
External Dependency
https://github.com/QiZhangNV/mixture-of-kittens/tree/qizhang/mcore-integration
Validation
1cdac4aa8d, including the MoE and determinism suites. The review follow-up at951fd137d5is running CI.Draft pending GPU validation. No performance result is claimed.