-
Notifications
You must be signed in to change notification settings - Fork 524
[OMNIML-4998] Per-expert weight quantization on TEGroupedLinear #1671
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
Changes from 10 commits
9003f0d
4af99c7
e3a6f5c
e8d9eea
1e11b4b
61c68d6
6751331
8fd0d53
c13ad0b
82b4979
d7ccf0a
26307c7
4c164f9
e4945a8
0bf4838
1080e68
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 |
|---|---|---|
|
|
@@ -680,6 +680,64 @@ class _QuantTELayerNormColumnParallelLinear( | |
|
|
||
| # Quantized subclasses to support TEGroupedMLP quantization | ||
| class _QuantMegatronTEGroupedLinear(_QuantTEGroupedLinear, _MegatronParallelLinear): | ||
| def _ep_group(self): | ||
| # Return the expert_model_parallel_group iff it's initialized AND has >1 rank. | ||
| # _MegatronTEGroupedMLP._setup populates parallel_state with the EP group; | ||
| # outside that wrapping it may be unset (e.g. ad-hoc unit tests). | ||
| ps = getattr(self, "parallel_state", None) | ||
| if ps is None: | ||
| return None | ||
| ep = ps.expert_model_parallel_group | ||
| if not ep.is_initialized() or ep.world_size() <= 1: | ||
| return None | ||
| return ep | ||
|
|
||
| def _gather_global_per_expert_amax(self): | ||
| # Return the global [num_gemms_global] per-expert amax, gathered across | ||
| # the EP group, without mutating the live weight_quantizer._amax buffer | ||
| # (the forward path indexes _amax by local expert position and would | ||
| # break if we replaced it with the global shape). | ||
| # | ||
| # Returns None when the layer isn't per-expert (per-tensor stays the | ||
| # legacy scalar path) or when EP == 1 (local == global, caller can | ||
| # reshape the live buffer directly). | ||
| if not self._is_per_expert_weight_quant(): | ||
| return None | ||
| quantizer = self.weight_quantizer | ||
| amax = getattr(quantizer, "_amax", None) | ||
| if amax is None: | ||
| return None | ||
| ep = self._ep_group() | ||
| if ep is None: | ||
| return amax.view(self.num_gemms) # EP=1: local IS global. | ||
|
|
||
| global_size = self.num_gemms * ep.world_size() | ||
| if amax.numel() != self.num_gemms: | ||
| raise AssertionError( | ||
| f"TEGroupedLinear weight_quantizer._amax numel {amax.numel()}; " | ||
| f"expected {self.num_gemms} (local per-expert) or " | ||
| f"{global_size} (global per-expert)." | ||
| ) | ||
| v_local = amax.view(self.num_gemms) | ||
| gathered = [torch.empty_like(v_local) for _ in range(ep.world_size())] | ||
| torch.distributed.all_gather(gathered, v_local, group=ep.group) | ||
| return torch.cat(gathered, dim=0) | ||
|
|
||
| def sharded_state_dict(self, prefix="", sharded_offsets=(), metadata=None): | ||
| # Gather the global per-expert amax ONCE before Megatron's save traversal. | ||
| # Stash on a temporary attribute that _process_quantizer_amax consumes; the | ||
| # live weight_quantizer._amax buffer stays local so forward keeps working. | ||
| # | ||
| # Done at the top of sharded_state_dict (not inside _process_quantizer_amax) | ||
| # so the EP collective completes BEFORE Megatron's dist-checkpoint save | ||
| # kicks off its own default-PG ALLGATHER metadata exchanges. Interleaving | ||
| # EP gathers with default-PG collectives deadlocks NCCL. | ||
| self._cached_global_per_expert_amax = self._gather_global_per_expert_amax() | ||
| try: | ||
| return super().sharded_state_dict(prefix, sharded_offsets, metadata) | ||
| finally: | ||
| self._cached_global_per_expert_amax = None | ||
|
|
||
| def _load_from_state_dict(self, state_dict, prefix, *args, **kwargs): | ||
| # _sharded_state_dict_grouped adds _extra_state{gemm_idx} for gemm_idx:[1, num_gemms] in | ||
| # sharded_state_dict which is same as _extra_state. The _extra_state{gemm_idx} is used for | ||
|
|
@@ -690,11 +748,52 @@ def _load_from_state_dict(self, state_dict, prefix, *args, **kwargs): | |
| for k, v in state_dict.items() | ||
| if not any(k.endswith(f"_extra_state{num}") for num in range(1, self.num_gemms)) | ||
| } | ||
| # Per-expert amax was saved as the gathered global [num_gemms_global] | ||
| # tensor. On restore each EP rank narrows it back to its local | ||
| # [num_gemms] slice before the base class's view_as() reshape. | ||
| ep = self._ep_group() | ||
| if ep is not None: | ||
| global_size = self.num_gemms * ep.world_size() | ||
| offset = ep.rank() * self.num_gemms | ||
| for k in list(filtered_state_dict.keys()): | ||
| if ( | ||
| "weight_quantizer" in k | ||
| and k.endswith("_amax") | ||
| and filtered_state_dict[k].numel() == global_size | ||
| ): | ||
| filtered_state_dict[k] = ( | ||
| filtered_state_dict[k] | ||
| .view(global_size) | ||
| .narrow(0, offset, self.num_gemms) | ||
| ) | ||
| return super()._load_from_state_dict(filtered_state_dict, prefix, *args, **kwargs) | ||
|
|
||
| def _get_shard_axis_dict(self, state_dict): | ||
| # The column-parallel parent marks weight_quantizer._amax as TP-sharded | ||
| # along axis 0 whenever weight_quantizer.axis is not None — correct for | ||
| # per-channel-along-output, but wrong for our per-expert path: the | ||
| # gathered amax is [num_gemms_global], not output-channel-sharded. | ||
| # Strip the marker so the dist-checkpoint framework treats it as | ||
| # replicated across TP. EP-aware handling lives in | ||
| # _gather_global_per_expert_amax (called from sharded_state_dict on | ||
| # save) and _load_from_state_dict (narrow on load). | ||
| shard_axis_dict = super()._get_shard_axis_dict(state_dict) | ||
| if self._is_per_expert_weight_quant(): | ||
| for k in list(shard_axis_dict): | ||
| if "weight_quantizer" in k and k.endswith("_amax"): | ||
| del shard_axis_dict[k] | ||
| return shard_axis_dict | ||
|
|
||
| def _process_quantizer_amax(self, k, v, quantizer_state_dict): | ||
| assert v.numel() == 1, "TEGroupedLinear only supports per-tensor quantization" | ||
| quantizer_state_dict[k] = v.view(-1) | ||
| # Per-expert weight amax: emit the gathered global tensor cached by | ||
| # sharded_state_dict's pre-pass. No collective fires here, so this | ||
| # call is safe to run inside Megatron's save traversal. | ||
| # Per-tensor (or non-weight) amax: just reshape the local value. | ||
| cached = getattr(self, "_cached_global_per_expert_amax", None) | ||
| if cached is not None and "weight_quantizer" in k and k.endswith("_amax"): | ||
| quantizer_state_dict[k] = cached.view(cached.numel()) | ||
| return | ||
| quantizer_state_dict[k] = v.view(v.numel()) | ||
|
Comment on lines
787
to
+796
Contributor
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. Scope the cached override to
Proposed fix- if cached is not None and "weight_quantizer" in k and k.endswith("_amax"):
+ if cached is not None and k == "weight_quantizer._amax":
quantizer_state_dict[k] = cached.view(cached.numel())
return🤖 Prompt for AI Agents |
||
|
|
||
| @QuantModuleRegistry.register( | ||
| {TEColumnParallelGroupedLinear: "megatron_TEColumnParallelGroupedLinear"} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -289,6 +289,7 @@ def _gpt_model_provider( | |
| use_cpu_initialization=meta_device, | ||
| num_moe_experts=num_moe_experts, | ||
| moe_grouped_gemm=moe_grouped_gemm, | ||
| sequence_parallel=(tp_size > 1), # Required for MoE + TP (mirrors hybrid path) | ||
| ) | ||
|
|
||
| if not meta_device: | ||
|
|
@@ -693,6 +694,55 @@ def test_te_grouped_vs_sequential_quantize(dist_workers_size_4, quant_cfg): | |
| ) | ||
|
|
||
|
|
||
| # OMNIML-4998: per-expert weight amax (axis=0) on TEGroupedLinear should round-trip | ||
| # through sharded_state_dict / dist-checkpoint with bit-for-bit equality across EP. | ||
| TE_GROUPED_PER_EXPERT_CFG = { | ||
| "algorithm": "max", | ||
| "quant_cfg": [ | ||
| # Disable everything (attention, embeddings, non-MoE MLP) so the test | ||
| # isolates the per-expert path on TEGroupedMLP experts. | ||
| {"quantizer_name": "*", "enable": False}, | ||
| # Re-enable axis=0 on TEGrouped MoE experts only -- this triggers the | ||
| # per-expert path inside _QuantTEGroupedLinear. | ||
| { | ||
| "quantizer_name": "*experts.linear_fc*.weight_quantizer", | ||
| "cfg": {"num_bits": 8, "axis": 0, "enable": True}, | ||
| }, | ||
| ], | ||
| } | ||
|
Comment on lines
+699
to
+712
Contributor
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. Re-enable the expert quantizers at the rule level. This config disables everything first with Proposed fix {
"quantizer_name": "*experts.linear_fc*.weight_quantizer",
- "cfg": {"num_bits": 8, "axis": 0, "enable": True},
+ "enable": True,
+ "cfg": {"num_bits": 8, "axis": 0},
},🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| def test_te_grouped_per_expert_sharded_state_dict(dist_workers, need_4_gpus, tmp_path): | ||
| """Per-expert (axis=0) weight amax round-trips through dist-checkpoint on TEGroupedMLP. | ||
|
|
||
| Mirrors ``test_moe_sharded_state_dict``'s setup exactly (dist_workers fixture, | ||
| hidden_size=256, tp=2 ep=2 etp=1 num_moe_experts=4 moe_grouped_gemm=True), with | ||
| only the quant_cfg varying. The existing test is known to pass at this layout | ||
| for FP8_DEFAULT_CFG / NVFP4_DEFAULT_CFG; this test exercises the OMNIML-4998 | ||
| per-expert (axis=0) path on top of the same infrastructure. | ||
| """ | ||
| moe_config = { | ||
| "tp_size": 2, | ||
| "ep_size": 2, | ||
| "etp_size": 1, | ||
| "num_moe_experts": 4, | ||
| "moe_grouped_gemm": True, | ||
| "transformer_impl": "transformer_engine", | ||
| } | ||
| dist_workers.run( | ||
| partial( | ||
| _test_sharded_state_dict, | ||
| tmp_path, | ||
| copy.deepcopy(TE_GROUPED_PER_EXPERT_CFG), | ||
| 256, | ||
| None, | ||
| False, | ||
| False, | ||
| moe_config, | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("ep_size", [1, 2]) | ||
| @pytest.mark.parametrize("sync_weight_amax", [True, False]) | ||
| def test_layer_sync_moe_local_experts_amax(dist_workers, ep_size, sync_weight_amax): | ||
|
|
||
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.
Include
(0,)in the TP sync allowlist._weight_axes_for_sync()detects both0and(0,), but it only appends0. The downstream check is an exactquantizer.axis in axes_for_sync, so tuple-normalized per-expert axes still skip TPamaxsync and can diverge across ranks.Proposed fix
if hasattr(module, "weight0"): quantizer = getattr(module, "weight_quantizer", None) if isinstance(quantizer, TensorQuantizer) and quantizer.axis in (0, (0,)): - return list(base_axes) + [0] + return [*base_axes, 0, (0,)] return base_axes🤖 Prompt for AI Agents