Repository navigation
[OMNIML-6071] Calibrate a distributed model on per-rank data #2740
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 all commits
8806f91
49f76a0
95bd7a7
47402e8
34fac65
0a2636e
c9e5235
08bc819
b6a145f
ebfb281
f991900
a360849
0849f61
666d032
90543d1
66a65b0
a34bfde
83c6937
f77b7e8
58308c8
fb6471e
beb2933
7a63bf4
bdbc3fe
a1a54d9
ae57933
e2137fc
aa060df
ce49b0e
49d238d
b71bc66
36f54d7
2174d52
9992833
f9aae8c
8d7daa5
74ddb3d
d4bc40b
01731d7
e1304e0
1f6b1e2
cea62b0
b1766f1
4861acb
d87f5af
7a6a2fa
cdc0dab
6eeb2fb
2c6f0af
3b940cb
367609d
6245fa7
afefa2a
359a064
f5d5468
8e811ae
1df1095
14bb439
ad6e140
dea9b79
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 |
|---|---|---|
|
|
@@ -2029,16 +2029,10 @@ def __init__(self, module): | |
| clip_ratios = [ | ||
| round(float(k), 2) for k in torch.arange(min_clip_ratio, 1.0, shrink_step) | ||
| ] + [1.0] | ||
| if self.is_per_tensor_clip(module): | ||
| self.loss = {k: torch.tensor(0.0, device=module.weight.device) for k in clip_ratios} | ||
| else: | ||
| self.loss = { | ||
| k: torch.zeros( | ||
| (co, math.ceil(ci / self.block_size)), | ||
| device=module.weight.device, | ||
| ) | ||
| for k in clip_ratios | ||
| } | ||
| shape = () if self.is_per_tensor_clip(module) else (co, math.ceil(ci / self.block_size)) | ||
| # One buffer for every candidate's loss, which the data-parallel sync reduces in place. | ||
| self.loss_buffer = torch.zeros(len(clip_ratios), *shape, device=module.weight.device) | ||
| self.loss = dict(zip(clip_ratios, self.loss_buffer)) | ||
| self.best_clip_val = None | ||
| self.best_loss = None | ||
|
|
||
|
|
@@ -2067,7 +2061,24 @@ def update_best_params(self): | |
| indices, self.awq_clip.w_amax * shrink, self.awq_clip.best_clip_val | ||
| ) | ||
|
|
||
| def sync_across_dp(module): | ||
| """Average the clip search over the data-parallel ranks, so every replica picks one clip. | ||
|
|
||
| Once, after the forwards: a module's forward count, as an MoE expert's, can differ by rank. | ||
| """ | ||
| group = module.parallel_state.data_parallel_group | ||
| if not group.is_initialized(): | ||
| return | ||
| losses = module.awq_clip.loss_buffer | ||
| dist.all_reduce(losses, op=dist.ReduceOp.SUM, group=group.group) | ||
| losses /= group.world_size() | ||
| tokens = torch.tensor(module.awq_clip.num_tokens, device=losses.device) | ||
| dist.all_reduce(tokens, op=dist.ReduceOp.SUM, group=group.group) | ||
| module.awq_clip.num_tokens = int(tokens.item()) | ||
|
Comment on lines
+2070
to
+2077
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. [SUGGESTION] Two smaller things in the same block:
Collaborator
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.
|
||
|
|
||
| def _clip_search(self, inputs, co_bsz=256, max_tokens=16): | ||
| if inputs.numel() == 0: # no tokens to search with; their loss, a mean over none, is NaN | ||
| return | ||
| weight = self.weight | ||
| self.weight_quantizer.enable() | ||
|
|
||
|
|
@@ -2126,11 +2137,6 @@ def _clip_search(self, inputs, co_bsz=256, max_tokens=16): | |
| # co_bsz, n_block | ||
| loss = (cur_out - org_out).float().pow(2).mean(dim=1) | ||
|
|
||
| parallel_group = self.parallel_state.data_parallel_group | ||
| if parallel_group.is_initialized(): | ||
| dist.all_reduce(loss, op=dist.ReduceOp.SUM, group=parallel_group.group) | ||
| loss /= parallel_group.world_size() | ||
|
|
||
| del cur_out, cur_w | ||
| self.awq_clip.loss[shrink][ | ||
| co_batch * co_bsz : min((co_batch + 1) * co_bsz, co) | ||
|
|
@@ -2198,6 +2204,11 @@ def postprocess(module): | |
| if module.awq_clip.is_input_quantized: | ||
| module.input_quantizer.enable() | ||
|
|
||
| # Every module's reduction before any weight gather, which an FSDP2 model's postprocess runs. | ||
| for module in model.modules(): | ||
| if is_quantized_linear(module) and hasattr(module, "awq_clip"): | ||
| sync_across_dp(module) | ||
|
Comment on lines
+2207
to
+2210
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. [SUGGESTION] Worth a comment here on what this loop does not cover under expert parallelism, since the new Per this PR's own contract ( The consequence is that an expert no token reaches at all on its owning rank keeps This is pre-existing (the
Collaborator
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. Leaving this out of this PR. The behavior is not specific to expert parallelism or to this change. In a single process, a module that no calibration token reaches also keeps The README line is about |
||
|
|
||
| for name, module in model.named_modules(): | ||
| if is_quantized_linear(module) and hasattr(module, "awq_clip"): | ||
| if module.awq_clip.num_tokens > 0: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,7 +53,7 @@ | |
| get_kv_cache_auto_quantize_config, | ||
| ) | ||
| from .kv_cache_auto_quant import _validate_search_inputs as _validate_kv_cache_search_inputs | ||
| from .mode import QuantizeModeRegistry, get_modelike_from_algo_cfg | ||
| from .mode import CalibrateModeRegistry, QuantizeModeRegistry, get_modelike_from_algo_cfg | ||
| from .nn import AnyQuantizer, QuantModule, SequentialQuantizer, TensorQuantizer | ||
| from .utils import is_quantized | ||
| from .utils.core_utils import _get_fsdp2_mesh | ||
|
|
@@ -72,6 +72,116 @@ | |
| "temporarily_fold_weights", | ||
| ] | ||
|
|
||
| # Calibration algorithms whose statistics stay on the rank that gathered them. Any other keeps its | ||
| # own there too when its config turns distributed_sync off. | ||
| _RANK_LOCAL_CALIBRATION = ( | ||
| "awq_lite", | ||
| "awq_full", | ||
| "svdquant", | ||
| "local_hessian", # its distributed_sync syncs only the max amax it starts from | ||
| "imatrix", | ||
|
shengliangxu marked this conversation as resolved.
Resolved
|
||
| ) | ||
|
Comment on lines
+75
to
+83
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. [IMPORTANT Compatibility] The refusal itself is well-founded —
This does not fire for users today — Two smaller things in the same block:
Collaborator
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. Both smaller points are taken in b517c75:
The changelog and README entries go with the PR that makes the refusal reachable. As you note, nothing outside
An entry here would announce a refusal that no user can reach until #2752. |
||
|
|
||
|
|
||
| def _rank_local_calibrations(algorithm: QuantizeAlgoCfgType) -> list[str]: | ||
| """The rank-local calibrations ``algorithm`` runs, with those it runs for its scales.""" | ||
| found = [] | ||
| for mode, options in get_modelike_from_algo_cfg(algorithm): | ||
| method = mode.removesuffix("_calibrate") | ||
| # The options as the calibration reads them, through its mode's schema: a string such as | ||
| # "false" is coerced, and a default counts as if spelled out. | ||
| config = CalibrateModeRegistry[mode].config_class.model_validate(options) | ||
| if method in _RANK_LOCAL_CALIBRATION: | ||
| found.append(method) | ||
| elif not getattr(config, "distributed_sync", True): # None skips the sync too | ||
| found.append(f"{method} with distributed_sync={config.distributed_sync}") | ||
| for key in ("weight_scale_algorithm", "scale_algorithm"): | ||
|
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.
[P2] Schema validation fixes the boolean-string case, but changing the sync check to
Collaborator
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. Right: the |
||
| if getattr(config, key, None) is not None: | ||
| found += _rank_local_calibrations(getattr(config, key)) | ||
| return found | ||
|
|
||
|
|
||
| def _group_ranks(group: dist.DistributedProcessGroup) -> list[int]: | ||
| """The ranks ``group`` spans, in its rank order; just this rank for a group not set.""" | ||
| if not group.is_initialized(): | ||
| return [dist.rank()] | ||
| return torch.distributed.get_process_group_ranks(group.group or torch.distributed.group.WORLD) | ||
|
|
||
|
|
||
| def _names_the_experts_groups( | ||
| state: "dist.ParallelState | None", plan: dist.DistributionPlan | ||
| ) -> bool: | ||
| """Whether ``state`` names the groups a routed expert of ``plan`` syncs its scales over. | ||
|
|
||
| Those of the ``(edp, ep)`` mesh the expert-parallel load lays the ranks out in: as its | ||
| expert-parallel group, this rank's row, whose ranks hold the experts' shards in row order, and | ||
| as its data-parallel group, its column, the ranks holding the same shard. | ||
| """ | ||
| if state is None or not state.expert_model_parallel_group.is_initialized(): | ||
| return False | ||
| row, column = divmod(dist.rank(), plan.ep_size) | ||
| expert_parallel = range(row * plan.ep_size, (row + 1) * plan.ep_size) | ||
| data_parallel = range(column, plan.world_size, plan.ep_size) | ||
| return _group_ranks(state.expert_model_parallel_group) == list(expert_parallel) and sorted( | ||
| _group_ranks(state.data_parallel_group) | ||
| ) == list(data_parallel) | ||
|
|
||
|
|
||
| def _refuse_unsupported_distributed_calibration( | ||
| model: nn.Module, algorithm: QuantizeAlgoCfgType | ||
| ) -> None: | ||
| """Refuse a calibration that would come out wrong on a model distributed over several ranks. | ||
|
|
||
| Each rank of such a model calibrates on its own data, so an algorithm that keeps its statistics | ||
| on the rank would calibrate the ranks' copies, or shards, of a weight differently. Under expert | ||
| parallelism each rank's per-expert quantizers hold its own experts, and calibration syncs them | ||
| over the groups their module's ``ParallelState`` names, every rank for a module converted | ||
| without one: a module holding routed experts must name the experts' expert-parallel group and, | ||
| for data parallelism, only the ranks holding the same experts, or any calibration would merge | ||
| different experts' scales. Called before anything changes, so the caller seeds the routed | ||
| experts' ``ParallelState`` before quantizing, as the expert-parallel load does; their quantized | ||
| modules, whose state calibration reads, are checked again once converted, before they calibrate. | ||
| """ | ||
| plan = dist.recorded_distribution_plan(model) | ||
| if plan is None or plan.world_size == 1: | ||
| return | ||
| refused = _rank_local_calibrations(algorithm) | ||
| if refused: | ||
| raise NotImplementedError( | ||
| f"Calibrating with {', '.join(refused)} is not supported on a model distributed over " | ||
| f"{plan.world_size} ranks. Each rank calibrates on its own data and this calibration " | ||
| "keeps what it gathers on the rank, so the ranks would calibrate their copies, or their " | ||
| "shards, of a weight differently. Calibrate with max or MSE with distributed_sync on, " | ||
| "GPTQ, SmoothQuant or awq_clip, or in a single process." | ||
| ) | ||
| if algorithm is None or not plan.is_expert_parallel: | ||
|
Comment on lines
+154
to
+157
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. [IMPORTANT ModeState] The guard checks the experts' groups but never checks that every other quantized module actually has a data-parallel group to sync over — so the exact failure this function exists to prevent still slips through for TP-sharded dense layers. Issue. The premise stated in the docstring ("Each rank calibrates on its own data, so an algorithm that keeps its statistics on the rank would calibrate the ranks' copies, or shards, of a weight differently") applies to any quantized module whose # modelopt/torch/quantization/plugins/huggingface.py:507
self._parallel_state = ParallelState(data_parallel_group=-1, tensor_parallel_group=tp_group)
Failure scenario. HF model TP-sharded over 2 ranks, Suggested fix. Extend the check: under a recorded plan with
Collaborator
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. Not changing this. The scenario needs a tensor-parallel dense module in a model with a recorded plan. No plan describes such a module, and no load in the series builds one:
Collaborator
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. Evidence for the reply above, from a run of this PR's expert-parallel test job on two CPU ranks (transformers 5.18):
The description also records the series' 4-GPU run at EP 2 (edp 2). It exported every scale bit-identical to EP 4, dense input scales included. The loads in this series do not build the layout this finding describes. |
||
| return | ||
| # Routed experts: the modules carrying an expert-parallel group, and the quantized modules under | ||
| # a container named ``experts``, as every expert-parallel plan in transformers names it. | ||
| holders = {} | ||
| for name, module in model.named_modules(): | ||
| state = getattr(module, "_parallel_state", None) | ||
| if (state is not None and state.expert_model_parallel_group.is_initialized()) or ( | ||
| isinstance(module, QuantModule) and "experts" in name.split(".") | ||
| ): | ||
| holders[name] = state | ||
| if not holders and not any("experts" in name.split(".") for name, _ in model.named_modules()): | ||
|
Comment on lines
+159
to
+168
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. [SUGGESTION] This walks holders, has_experts = {}, False
for name, module in model.named_modules():
under_experts = "experts" in name.split(".")
has_experts |= under_experts
state = getattr(module, "_parallel_state", None)
if (state is not None and state.expert_model_parallel_group.is_initialized()) or (
isinstance(module, QuantModule) and under_experts
):
holders[name] = state
if not holders and not has_experts:
return # no routed experts here, as in a VLM's vision tower: no experts' scales to mergeAlso,
Collaborator
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. Leaving it: the second walk runs only when the first finds no holder, since |
||
| return # no routed experts here, as in a VLM's vision tower: no experts' scales to merge | ||
|
Comment on lines
+161
to
+169
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. [SUGGESTION] The "no routed experts here" escape hatch, and the refusal it guards, both key off the module names
Both shapes come from the same root cause. The cheapest fix for (1) is to skip the whole EP block when no quantizer is enabled (there is nothing to sync); for (2), noting in the docstring that the check assumes it is handed the model that carries the routed experts, not a slice of it, would at least make the assumption explicit.
Collaborator
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. Postponed: neither shape arises in the stack's flows. The VLMs given expert-parallel specs (Kimi-VL, Kimi-K2.5, Kimi-K3) keep their routed experts in the language model, so hf_ptq's all-disabled quantize of the other towers passes the name probe, and the EP loader in [9/21] seeds every routed expert's
Comment on lines
+168
to
+169
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. [SUGGESTION] The "no routed experts here" escape hatch keys on the literal name segment In practice #2741's loader is what sets both the plan and the groups, so the gap is narrow today, but the fallback inverts the safe default. Consider keying the escape on something structural instead — e.g. refuse when the plan is expert-parallel and no module carries an expert-parallel group, unless the caller opted out explicitly (the vision-tower case
Collaborator
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. Keeping the name-based escape. Every expert-parallel plan in transformers 5.18 (52 of them, granitemoe_swa's
Comment on lines
+159
to
+169
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. [SUGGESTION] The
Collaborator
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.
|
||
| wrong = [name for name, state in holders.items() if not _names_the_experts_groups(state, plan)] | ||
|
Comment on lines
+161
to
+170
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. [SUGGESTION] Under an expert-parallel plan with
That this is a live foot-gun rather than a hypothetical is recorded in the PR description itself: the tenth-round note says the new GPU test's dense linears need the seeded data-parallel group and that "seeding them without that group fails the test: Cheap to close in the same pass over holders, dense = {}, []
for name, module in model.named_modules():
state = getattr(module, "_parallel_state", None)
if (state is not None and state.expert_model_parallel_group.is_initialized()) or (
isinstance(module, QuantModule) and "experts" in name.split(".")
):
holders[name] = state
elif isinstance(module, QuantModule) and state is not None:
dense.append((name, state))
...
# A module replicated on every rank syncs its scales over all of them, not over an expert column.
misplaced = [
name
for name, state in dense
if state.data_parallel_group.is_initialized()
and len(_group_ranks(state.data_parallel_group)) != plan.world_size
]
if misplaced:
raise NotImplementedError(
f"Calibrating a model whose experts are sharded over {plan.ep_size} ranks needs every "
f"module outside the routed experts to name all {plan.world_size} ranks as its "
"data-parallel group, since every rank holds a copy of it, or calibration would give "
f"the mesh's rows different scales. These modules do not: {misplaced[:3]}."
)Non-blocking: no in-tree loader seeds an expert-parallel plan on this base yet (#2741 is the first), so only the hand-seeded tests reach it today — which is also why it is cheaper to pin now than to find later.
Collaborator
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. This check is tracked as a follow-up, because it does not fit this PR's size budget. The series' load already seeds the layout that the check would require:
The tenth-round note records a mutation I made by hand to show that the test catches a missing dense sync. No loader produces that layout. |
||
| if not holders or wrong: | ||
| raise NotImplementedError( | ||
| f"Calibrating a model whose experts are sharded over {plan.ep_size} ranks needs each " | ||
| "routed expert's ParallelState to name, as its expert-parallel group, this rank's row " | ||
| f"of the ({plan.edp}, {plan.ep_size}) expert-parallel mesh and, as its data-parallel " | ||
| f"group, its column, the {plan.edp} rank(s) holding the same experts, or calibration " | ||
| "would sync their scales over the wrong ranks. " | ||
| + ( | ||
|
Comment on lines
+157
to
+178
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. [SUGGESTION] The expert-group branch is sound when it runs from That is deliberate and tested ( Since the post-conversion check in def _refuse_unsupported_distributed_calibration(model, algorithm, check_experts=True):
...
if algorithm is None or not plan.is_expert_parallel or not check_experts:
returnwith
Collaborator
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. Keeping the check before conversion. On this series' path, the early check reads the real groups:
No in-tree Hugging Face plugin seeds routed experts in |
||
| f"These modules do not: {wrong[:3]}." | ||
| if wrong | ||
| else "None of the model's modules carries an expert-parallel group." | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| def _refuse_awq_on_a_sharded_fsdp2_model( | ||
| model: nn.Module, algorithm: QuantizeAlgoCfgType, candidate: str | None = None | ||
|
|
@@ -128,6 +238,7 @@ def calibrate( | |
| Returns: The calibrated pytorch model. | ||
| """ | ||
| _refuse_awq_on_a_sharded_fsdp2_model(model, algorithm) | ||
| _refuse_unsupported_distributed_calibration(model, algorithm) | ||
| if forward_loop is not None: | ||
| # get the number of arguments of forward_loop | ||
| num_args = len(inspect.signature(forward_loop).parameters) | ||
|
|
@@ -502,6 +613,7 @@ def forward_loop(model) -> None: | |
| """ | ||
| quantize_config = QuantizeConfig(**dict(config)) | ||
|
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.
[P2] The allowed EP flow still loses its transport during the conversion immediately below.
Collaborator
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 enabling change is #3074, and it is already underneath every PR that can produce an expert-parallel model.
This PR's loop and guard take whatever implementation the load selected and set none, so its own code and tests do not need #3074. Re-basing it on #3074 would put #3074's diff in this PR.
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.
The reply specifies #3074-first merge order, but the enabling fix is not present at this head.
Collaborator
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. This PR now sits on #3074 ( |
||
| _refuse_awq_on_a_sharded_fsdp2_model(model, config.get("algorithm")) | ||
| _refuse_unsupported_distributed_calibration(model, config.get("algorithm")) | ||
| if not is_quantized(model): | ||
| model = apply_mode(model, mode=[("quantize", dict(config))], registry=QuantizeModeRegistry) | ||
| else: | ||
|
|
@@ -940,6 +1052,14 @@ def forward_backward_step(model, batch) -> None: | |
| if isinstance(quantization_formats, str) or not isinstance(quantization_formats, Sequence): | ||
| raise TypeError("`quantization_formats` must be a sequence of formats.") | ||
| quantization_formats = list(quantization_formats) | ||
| plan = dist.recorded_distribution_plan(model) | ||
| if plan is not None and plan.is_expert_parallel: # before anything converts the model | ||
| raise NotImplementedError( | ||
| f"auto_quantize does not support a model whose experts are sharded over {plan.ep_size} " | ||
| "ranks yet: its forwards would run the experts without the expert-parallel forwards " | ||
| "that quantization replaces and distributed_forward_loop restores. Quantize it with " | ||
| "mtq.quantize through distributed_forward_loop, or AutoQuantize in a single process." | ||
| ) | ||
|
|
||
| method_name, method_config = _parse_auto_quantize_method(method) | ||
| is_kv_search = constraints is not None and constraints.get("cost_model") == COST_MODEL_KV_CACHE | ||
|
|
@@ -1061,6 +1181,7 @@ def forward_backward_step(model, batch) -> None: | |
| ) | ||
| # Every candidate, before the search calibrates any: calibrate() would refuse mid-search. | ||
| _refuse_awq_on_a_sharded_fsdp2_model(model, algo, candidate=name) | ||
| _refuse_unsupported_distributed_calibration(model, algo) | ||
|
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.
[P2] The acknowledged EP AutoQuantize issue remains open despite being tracked as a follow-up. A correctly seeded EP model with an FP8/max candidate passes this check, then Could wrapper preservation be handled at conversion using the existing
Collaborator
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. Taken in fbbe227. Preserving the wrapper at conversion would change how DynamicModule conversion keeps transformers' instance forwards, which reaches beyond this PR. The refusal can be lifted once that lands. The new two-rank test,
It fails without the refusal. |
||
|
|
||
| # Select the appropriate searcher based on method | ||
| if method_name not in AUTO_QUANTIZE_SEARCHERS: | ||
|
|
||
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.
[IMPORTANT Algorithm]
num_tokensis reduced over the data-parallel group only, so a module no token reached on any rank of its DP column ends withnum_tokens == 0,postprocessis skipped at line 2213, andAWQClipHelper.__init__'smodule.weight_quantizer.disable()is never undone — the module silently exports unquantized (and itsinput_quantizerstays disabled too). Nothing catches it:_check_weight_quantization_took_effectruns before calibration, and the newinputs.numel() == 0early return in_clip_searchturns what used to be a NaN/garbage loss into a clean, silent skip.Why it matters more now: this PR's whole premise is that each rank calibrates on its own slice, so each module sees ~1/world_size of the tokens and a zero-token expert becomes a realistic outcome rather than a pathological one. A fused-MoE checkpoint with one expert missing its
weight_scaleis rejected by the TRT-LLM / vLLM MoE loaders, and under EP the skip is rank-dependent (same module path, different experts), so the ranks disagree on whether that module is quantized at all.max_calibratealready solves the analogous problem for input amax and is worth reusing rather than paralleling: lines 459-482 share an expert's input amax over the expert-parallel group precisely "so experts that received no tokens take the layer's value", and_check_moe_calibration_complete(line 483-487) then fails loudly on what is still missing. Suggest either reducingnum_tokensoverexpert_model_parallel_groupas well, so a row agrees, or — at minimum, per CONTRIBUTING's "fail loudly rather than silently ignore" — emitting awarn_rank_0naming the modules awq_clip left disabled, so the user does not discover it at deploy time.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.
The premise that per-rank data makes a zero-token module more likely does not hold. Each module's summed count covers the whole calibration set, as in a single process:
So an expert ends at
num_tokens == 0only if no token of the whole calibration set was routed to it. That is the same condition as in a single-process run on the same data, and main'sawq_clipskips such a module the same way.Summing the count over the expert-parallel group would be wrong here, because that group holds different experts. An expert that no token reached would then run
postprocesson all-zero losses and take the first candidate, the smallest clip.max_calibratecan share input amax over that group because a layer's experts see the same inputs, but a clip search scores each expert's own weight.A warning for the modules
awq_clipleaves disabled would help with that pre-existing case, but it is a separate change, so I'm leaving it out of this PR. Note thatwarn_rank_0would miss experts held on other ranks.