[model, quant] fix: Preserve quantized expert scale dimensions - #5745
Merged
Conversation
Signed-off-by: Yu Yao <yaoyu.094@gmail.com>
Contributor
Author
|
/ok to test a0886f0 |
Contributor
|
Light review — LGTM with one doc nit The fix is correct.
Suggested test cases
No perf tests impacted. |
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Contributor
Author
|
/ok to test 5374f9f |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
AutoBridge.export_hf_weights_quant()can silently drop a valid singleton block-grid dimension from MoE expert scale tensors. The trigger is a selected expert weight whose block quantizer returns a scale shaped[block_rows, block_cols, 1]with either block-grid axis equal to one, which is valid when an expert shard dimension equals the caller-selected block size.For grouped EP=2 export, two local scales shaped
[1, 2, 1]are staged as[2, 1, 2, 1], but the current helper emits[2, 2, 1]. Grouped accumulation then preserves that malformed geometry in the Hugging Face*_scale_invtensor. Per-expert and EP=1 paths can lose the same quantizer-owned axis.This affects the quantize-before-gather workflow introduced by #2737. The adjacent fix in #5731 established that ordinary EP gathering must remove only its own staging axis, but the quantization-specific scale helper retained the unqualified squeeze.
Root cause and fix
gather_from_ep_ranks_scale()stages tensors with a leading EP-owned dimension, then calls unqualifiedsqueeze()and re-adds only a trailing dimension. That removes every singleton axis, including valid block-grid axes owned by the quantizer.Use
squeeze(0)so only a size-one staging axis is removed. For grouped EP greater than one, the leading EP axis remains available to_accumulate_grouped_export()as designed.Regression evidence
The focused test executes the real helper with a deterministic two-rank gather and a
[1, 2, 1]local scale.Fail before:
Pass after, unchanged test:
Adjacent validation:
Both repository checks passed. The focused CPU environment used the repository's pinned MCore revision and disabled only unrelated optional CUDA/Triton imports.
Scope
This changes one internal tensor-axis operation and adds one focused CPU regression test. It does not change conversion APIs, quantization formats, model registrations, dependencies, workflows, the lockfile, or Megatron-Core.