Repository navigation
Conversation
Contributor
[Medium impact] No newly introduced blocking issue was found in the changes since the previous review. SummaryCompletes the reference MXFP4 quantization path and adds custom-recipe calls for grouped forward GEMMs.
Reviews (13) · Last reviewed commit: "fix(custom_recipes): reach the grouped-t..." · Reviewed by Greptile |
Member
|
@NolanHo Please sign every commit in this PR. The DCO check is failing because commits are missing a |
NolanHo
force-pushed
the
feat/mxfp4-custom-recipe-grouped
branch
from
November 6, 2026 19:40
2e8bf88 to
6658007
Compare
CustomRecipe 目前只能用于普通 Linear:`general_gemm` 有自定义分流,而 `general_grouped_gemm` / `general_grouped_gemm_for_grouped_tensor` 没有,因此 MoE 专家(走 `GroupedLinear`)永远无法到达 quantizer 的 `qgemm`。同时参考实现里 block-32 / E8M0(即 MXFP4)那条分支未写完,即使能走到也会中途报错。 本补丁按实测失败逐项修复(每项都有可复现的报错原文): reference_nvfp4.py 1. pow_2(MXFP4)分支只做了配置校验,从未准备 `row_input` / `col_input` / `global_amax_row` / `global_amax_col`,公共尾部因此抛 `UnboundLocalError: cannot access local variable 'row_input'`。 补上赋值,并对 `_split_quantization` 可能产生的 0 元素分片加保护 (`torch.max()` 无参调用在空张量上会抛 `RuntimeError: max(): Expected reduction dim to be specified`)。 2. `size(*args, **kwargs)` 忽略参数、始终返回整个逻辑形状,导致调用方 `GroupedLinear._validate_or_alloc_output` 的 `rows = x.size(0)` 得到 `torch.Size`, `torch.empty((rows, cols))` 抛 `TypeError: empty(): argument 'size' failed to unpack the object at pos 2`。 改为 `size(dim=None)`,传入 dim 时返回该维长度。 3. `update_usage` 在两个方向都没有数据(占位张量)时直接抛 `Cannot generate FP4 data, even from FP4 data transpose`;占位符没有任何可推导的 数据,应视为 no-op 而不是错误。 4. 补 `quantize_()`。`module/base.py::quantize_weight` 在 workspace 暴露 `quantize_` 时走它,否则回落 `tex.quantize(...)`——后者无法驱动 Python 参考 量化器,workspace 会保持为空,随后 GEMM 拿到 `data is None`。 cpp_extensions/gemm.py 5. 给 `general_grouped_gemm` 增加自定义分流(列表形态:A 为权重列表、B 为激活列表、 out 为单张量、`m_splits` 为每个专家的有效激活行数),逐组调用 `qgemm`。 6. 给 `general_grouped_gemm_for_grouped_tensor` 增加自定义分流 (`GroupedTensorStorage` 形态,按 `offsets` / `scale_inv_offsets` 切组)。 两处分流都对不支持的契约显式拒绝(GELU 输出、bias 梯度、D_dtype、输出量化、 grad),而不是静默产生错误结果。 实测背景:在 B300(sm_103,EP8+CP8)上,MXFP4 blockscaled fprop GEMM 已用 CUTLASS `KernelTmaWarpSpecialized1SmBlockScaledMxNvf4UltraVs32Sm103` 实现并与参考实现 逐位一致;上述缺口是它无法接入训练的原因。 Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
for more information, see https://pre-commit.ci Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
…ed custom GEMM TEGroupedLinear passes one output tensor per expert rather than a single concatenated buffer, so refusing single_output=False made the MoE expert path unusable even with the dispatch in place. Both layouts are now handled: with a single buffer each expert writes rows [offset : offset+m_splits[i]], and with a list each expert writes its own tensor. Verified on a TEGroupedLinear with four experts: the dispatch is reached and every group is dispatched to qgemm. Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
for more information, see https://pre-commit.ci Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
A custom (reference) quantizer cannot be cached safely. The cached entry is a TensorSpec whose storage
is driven by update_usage, and a gradient-checkpoint replay can reach the GEMM after that storage has
been released, which surfaces as
FPROP GEMM: quantized weight data (B.data) is None
in a model whose single un-replayed forward is fine. Measured on DSV4.1-Flash (40 layers, EP8/CP8) with
an MXFP4 CustomRecipe: identical weights and identical forward, and the failure appears only on the
replay path (checkpoint.py recompute_fn -> transformer_layer._run_mlp), where the operand handed to
the GEMM is a payload-less reference tensor of the right shape while the module's own parameter is a
healthy bf16 Parameter.
Skipping the workspace path for custom quantizers re-quantises per call instead. With this in place the
same run completes a full training step (loss 3.667, grad_norm 1.823) with gradient checkpointing left
ON, which is what keeps activation memory within budget.
Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
NolanHo
force-pushed
the
feat/mxfp4-custom-recipe-grouped
branch
from
November 6, 2026 19:42
6658007 to
4d83b57
Compare
… custom GEMM The grouped entry point issues one single-group GEMM per expert group. On a real training step that is ~40,000 kernel launches for ~1,000 GEMMs of work. The per-launch cost is small enough that it never surfaces in a per-phase profile, but the sum dominates the custom-recipe path's remaining overhead versus the native FP4 path. Recorded at the loop rather than fixed: collapsing it requires a grouped GEMM kernel, which is a separate change with its own numerical acceptance test. Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
NolanHo
force-pushed
the
feat/mxfp4-custom-recipe-grouped
branch
from
November 7, 2026 12:20
54d5879 to
9d083a9
Compare
Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
NolanHo
force-pushed
the
feat/mxfp4-custom-recipe-grouped
branch
from
November 8, 2026 18:28
64556bb to
15bdef5
Compare
Contributor
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
…gemm contracts The grouped dispatch called is_custom on the GroupedTensorStorage containers that general_grouped_gemm_for_grouped_tensor receives, and is_custom raises on any type that is neither a Quantizer/QuantizedTensorStorage nor a Tensor. Every grouped-tensor GEMM, native ones included, failed before reaching the native implementation. The dispatch now inspects the quantizer (or the discrete list members), so an ordinary grouped tensor keeps taking the native path. The per-expert loop also handed qgemm only the packed data and scales: it dropped the per-group bias that use_bias=True promises, omitted qresult_x/qresult_w (so a non-pow-2 NVFP4 recipe, which derives its global scale from them, could not run), and ran the fprop computation for grad=True while every shape check still passed. The bias now goes to qgemm the way custom_gemm passes it, the quantized operands travel with the packed data, and dgrad/wgrad are refused explicitly instead of returning a wrong gradient. NVFP4TensorRef.quantize_ copied only the packed data and block scales, leaving global_amax_row/global_amax_col from the previous quantization, and ignored noop_flag. It now delegates to the quantizer's update_quantized, which is what QuantizedTensorStorage.quantize_ does: every field a later GEMM reads is refreshed, and the replay skip flag is honoured. Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
NolanHo
force-pushed
the
feat/mxfp4-custom-recipe-grouped
branch
from
November 8, 2026 18:34
f515320 to
2e556c9
Compare
…dvertise The custom grouped-tensor GEMM read its operands backwards. `A` is the per-expert weight and `B` the per-expert activation at every `layout="TN"` call site (module/grouped_linear.py, ops/basic/grouped_linear.py, tests), but the helper fed `A` in as the activation and used its row count for the output segment, so a custom recipe would either trip the segment check or compute the transposed product. The activation slice, its scales and the row count now come from `B`. That layout has no per-group QuantizedTensorStorage to give a quantizer as `qresult_x`/`qresult_w`, so a non-pow-2 recipe is refused inside `qgemm`; the comment above the helper now says so instead of leaving a reader to infer it. The list grouped GEMM now length-checks the per-group bias before the loop that mutates the caller's buffer; a short list used to IndexError after some groups were already written. NVFP4QuantizerRef.qgemm refused any bias, so a biased custom recipe could not run at all; it adds the bias in the float32 epilogue now, which is where the native path puts it, so grouping no longer decides whether bias works. NVFP4TensorRef.quantize_ delegates to `update_quantized`; that path flattens only ndim > 2 and skips the dtype guard `quantize` applied, so the reference method restores both. Its docstring described a call site that the weight-cache change removed, so it now states the contract instead of narrating a caller. A test pins the classification the previous commit fixed: a plain GroupedTensorStorage must not be routed through `is_custom`. Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
…s body The new test was inserted before `test_swizzle_scales_and_pack_ptrs_for_discrete_weights` finished, so that test's scale-poisoning checks ended up inside the new function, where `swizzle_type`, `shape` and `scale_tensors` do not exist, and the original test lost those checks. Moved to the end of the file: the swizzle test is whole again and the new function only touches its own locals. Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
… the custom grouped paths `_custom_grouped_tensor_gemm` read the block scales as `rowwise_scale_inv`, a field GroupedTensorStorage does not have — it stores them as `scale_inv` — so every group died on "missing rowwise scale_inv for a custom quantizer" as soon as the dispatch above it stopped raising. Both names are accepted now, keyed off the buffer the storage actually carries. Nothing covered this. The module and ops grouped paths only build custom operands for the discrete (list) form, so the reference `qgemm` implementations were never exercised by the suite and the defect above only surfaced once the path was driven by hand on a device. The new `tests/pytorch/test_custom_recipe_grouped.py` drives both entry points with the reference NVFP4/MXFP4 quantizer and pins what has no other coverage: per-expert operand roles and m_splits slicing, per-group offsets and block scales, the per-expert bias, the `qresult_x`/`qresult_w` hand-off that non-pow-2 NVFP4 needs, the documented refusals (dgrad/wgrad, non-TN layout, malformed bias list, non-pow-2 recipe on the grouped-tensor layout), and the reference quantizer's in-place update and fp32-epilogue-bias contracts. Signed-off-by: Nolan Ho <kujyo.eia.serias@gmail.com>
Author
|
@ptrendx Sign-off is in place on every commit and DCO is green on the current head. I also worked |
This branch has not been deployed
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.
What this changes
Makes
CustomRecipeusable beyond plainLinear, and completes the MXFP4 (block-32 / E8M0) path inthe reference implementation. Without these, a custom quantizer can never be used for MoE experts and
the reference MXFP4 branch cannot run at all.
general_gemmalready dispatches to a custom quantizer'sqgemm; neither grouped GEMM entry does,so
GroupedLinear— the MoE expert path — goes straight tonvte_grouped_gemmwith native tensors.Changes, each tied to an observed failure
pytorch/custom_recipes/reference_nvfp4.pypow_2_scales(MXFP4) branch is unfinished. It validates the configuration and then fallsthrough to the common tail, which raises:
UnboundLocalError: cannot access local variable 'row_input' where it is not associated with a valueThe source's own
# TODO(etsykunov): Fix bug where global_amax_row and global_amax_col are not definedpoints at the same hole. This preparesrow_input/col_inputand both amax values onthat branch.
_split_quantizationcan hand this a 0-element split, wheretorch.max()without a reductiondim raises
RuntimeError: max(): Expected reduction dim to be specified for input.numel() == 0.Guarded.
size(*args, **kwargs)ignores its argument and always returns the full logical shape, sox.size(0)yields atorch.Size.GroupedLinear._validate_or_alloc_outputdoestorch.empty((rows, cols))withrows = x.size(0)and dies withTypeError: empty(): argument 'size' failed to unpack the object at pos 2 with error "type must be tuple of ints, but got torch.Size".Changed to
size(dim=None), returning that dimension when asked. The no-argument behaviour isunchanged.
update_usagerefuses a placeholder. A tensor with neitherdatanordata_tpopulated hasnothing to generate from, yet TE raises
RuntimeError: Cannot generate FP4 data, even from FP4 data transpose. Treated as a no-op instead.quantize_().module/base.py::quantize_weightcallsworkspace.quantize_(...)when theworkspace exposes it and otherwise falls back to
tex.quantize(...), a C++ entry that cannot drivea Python reference quantizer — the workspace stays empty and the GEMM then fails on
data is None.Implemented in Python.
pytorch/cpp_extensions/gemm.pygeneral_grouped_gemm(list form): per expert, the activation rows areB[i][0:m_splits[i]], theweight is
A[i], and the result goes toout[0][offset : offset+m_splits[i]]. The operand rolesare taken from
GroupedLinear's own call site (grouped_linear.py), which passes(weights_fp8, inputmats, [out], single_output=True, m_splits=...).general_grouped_gemm_for_grouped_tensor(GroupedTensorStorageform): groups are sliced fromoffsets/scale_inv_offsets/rowwise_data/rowwise_scale_inv.Both refuse contracts they cannot honour explicitly (GELU outputs, bias gradients,
D_dtype, outputquantisation,
grad) rather than producing a silently wrong result.Testing
Measured on 8× sm_103 (B300), EP8 + CP8, with an E2M1/block-32/E8M0 CUTLASS kernel
(
KernelTmaWarpSpecialized1SmBlockScaledMxNvf4UltraVs32Sm103) bound throughCustomRecipe.qgemm:qgemmmatches this reference implementation bitwise on real quantised operands(
max|Δ| = 0.0, reference magnitude ≈ 12.6) for the fprop direction;the observed messages.
Not exercised by this PR:
grad/dgrad/wgrad for the grouped paths (explicitly refused), androw_scaled_nvfp4in the MXFP4 configuration (rejected upstream already).