Skip to content

[moe_training] Fused dual-axis MXFP8 quantization with correct metadata - #4995

Merged
vkuzo merged 2 commits into
mainfrom
fused-mxfp8-dual-axis-quantization
Oct 15, 2026
Merged

vkuzo merged 2 commits into
mainfrom
fused-mxfp8-dual-axis-quantization

Conversation

@sanketpurandare

@sanketpurandare sanketpurandare commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Performnace of this fused cute-dsl quantizer is better than cuda, a single API for dense and grouped mxfp8 linears, no need for scale rearrangements since swizzled layouts are directly output.

AI-assisted implementation details:

Summary

Preserve Protonu's original authored kernel from #4895 and consolidate the correctness fixes into this single dependency. Credit also to Aleksandar Samardzic's #4702 for independently addressing the single-axis FakeTensor metadata issue. Neither original PR is modified.

The public/custom-op/fake signatures now match the surrounding APIs: (x, block_size=32, scaling_mode="rceil", offs=None). Dense offs=None specializes the same kernel with compact blocked scales and no offset processing. Tensor offsets preserve grouped layouts, empty experts and uncovered tails. The API quantizes BF16 activations/output gradients; AO grouped-GEMM wiring remains separate. TorchTitan #5334 is the consuming change.

Fixed the RCEIL scale0/1 cutoff to 448*2^-127 (0x04600000) and retained current NOSAT Inf/NaN semantics. Numerical tests now use direct to_mx(RCEIL) on both axes with per-group unblocking, including exact boundary neighbors and both CTA paths. Removed the low-value input-validation test and redundant standalone determinism test; repeated dirty-buffer writes retain meaningful coverage.

Supplied destinations are checked for shape/dtype/device/contiguity/alignment and overlapping byte intervals; disjoint arena views remain supported. Grouped compilation explicitly enables device assertions for endpoint ordering/bounds because CuTeDSL disables them by default. Removed the unreachable CTA branch and redundant reciprocal convention.

Executed local verification

  • 63 affected fused/single-axis CUDA tests passed, including real/fake schemas, actual Inductor compile, empty/tail/dirty cases, endpoint failure controls and nondefault cross-device stream.
  • Repository Ruff 0.11.6 check/format and strict public API documentation builds passed.
  • Exact TorchTitan consumer: 69 CPU tests, 32 Linear GPU tests and 6 real two-rank FSDP lifecycle tests passed (compiled RAF, retained PP weights and CUDA-graph replay).

Matched GB300 BF16 CUDA-graph replay, seven rotated timing rounds, 100ms target per measurement:

Input MxN Dense CuTeDSL (us) Current dense CUDA + two scale rearrangements (us) Ratio
4096x7168 18.306 27.510 1.50x
8192x7168 36.279 49.526 1.37x

Parsed replay traces contain one CuTeDSL kernel versus the CUDA quantizer plus two swizzle kernels, with nonzero durations and no early collection stop. These are cast timings, not training-step speedups. Raw timing samples include round-to-round variation.

External review and new CI remain pending; no landing-readiness claim.

Quantizing a 2D tensor along both axes currently takes three kernels and two
full reads of the same tensor: the 1x32 CuTeDSL cast along N, the 32x1 CUDA cast
along M, and a standalone Triton K-groups swizzle of the colwise scales. Add a
CuTeDSL kernel that reads the input once and emits all four outputs, with both
scale tensors already in the blocked layouts their consumers expect. Byte
accounting drops from 6.125 to 4.0625 bytes per input element.

The kernel puts one CTA on each (128, 128) input tile, stages it into SMEM with
cp.async, and runs the rowwise and colwise passes sequentially at full CTA
width. The arithmetic is packed 16-bit throughout -- max.u16x2 on sign-masked
bf16 for amax, cvt.rn.satfinite.e4m3x2.bf16x2 for the cast -- and prmt.b32 byte
permutes keep the logically transposed colwise store fully coalesced. bf16 and
rceil only; both dimensions and every group size must be multiples of 128.

Exposed as mxfp8_quantize_2d_1x32_32x1_cutedsl, registered as a custom op with a
fake impl, alongside the two kernels it composes.

Uncovered tail. offs[-1] < total_M is supported and is the norm rather than a
corner case: a dropless MoE dispatcher sizes its buffer for worst-case padding
and fills only a prefix. Rows past offs[-1] are still quantized, but their
colwise scale bytes are zeroed to match the K-groups swizzle, which iterates
groups and returns new_zeros. Falling through to the placement math instead aims
the store into group 0's slab and races every n-tile CTA on one 512-byte line.

Bug-compatible where the two kernels it replaces disagree with each other, since
the point is to be a drop-in:

  * A block whose amax is subnormal enough to reach E8M0 scale byte 0 gets a
    2**127 multiplier on both outputs, matching the 254 - scale_biased
    reciprocal both use.
  * Inf: the rowwise kernel converts NOSAT, so Inf poisons the block to scale
    255 and all-NaN data; the colwise CUDA kernel converts SATFINITE, so Inf
    clamps to scale 254 with finite data. Both are reproduced.

The one deliberate divergence is NaN on the colwise output. The CUDA kernel
drops NaN from its amax entirely -- an all-NaN block still gets the scale its
finite elements imply -- and reproducing that would need a per-element NaN test
in the hottest loop in order to imitate a behavior that hides NaN corruption
rather than surfacing it. The fused kernel poisons instead, and the test asserts
the difference so it fails loudly if either side changes.

Tests compare raw bytes at atol=0/rtol=0 against the three-kernel composition,
not against to_mx, because being a drop-in for that composition is the contract.
They cover shapes and group counts, uncovered tails including the zero-fill,
degenerate and mixed-magnitude blocks, Inf/NaN, dirty destination buffers so a
never-written region cannot hide behind a zeroed page, determinism across
repeated launches, and input validation. The rowwise half is additionally
checked against the shared MXFP8 semantic contract.

The benchmark follows the existing 1x32 and 32x1 scripts, but times both sides
under CUDA-graph replay rather than eagerly. torchao's triton_op dispatch costs
on the order of 100us per call regardless of problem size and the baseline pays
it three times, which timed eagerly swamps every shape below roughly 0.5 GB and
reports 15x on small shapes -- launch overhead, not a kernel win. It also checks
that the kernel does not capture as an empty graph, which is what happens when a
CuTeDSL kernel ignores the caller's stream and which would read as a 100x win.
Geomean is 2.6x across the swept shapes, 2.0x on the largest, at up to 95% of
the measured SM bandwidth roof on a GB200 at 2062 MHz.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pytorch-bot

pytorch-bot Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/4995

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 86f7e2a with merge base 9d0c494 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 12, 2026
@sanketpurandare sanketpurandare added the module: training quantize_ api training flow label Oct 12, 2026
sanketpurandare added a commit to pytorch/torchtitan that referenced this pull request Oct 12, 2026
## Agent details

Prepared with AI assistance.

Reuse TorchAO's fused rowwise/columnwise activation cast and existing single-axis CuTeDSL casts, consuming blocked E8M0 scales without separate rearrangement kernels. Keep CUDA as the default and expose CuTeDSL as an explicit activation_quantization_backend option through both the linear and its converter. Input-save format, weight quantization, output ownership, FSDP lifetime and gradient accumulation policy are unchanged.

CuTeDSL requires 128-aligned BF16 matrices and the required runtime/API at construction. Unsupported shapes fail explicitly rather than padding activations to a larger GEMM or silently selecting another implementation. Represent dense matrices as one group, view the valid column-scale prefix, and retain immutable real endpoints for CUDA-graph pointer lifetime. Fake endpoints never enter the real cache.

Explicitly request RCEIL for every activation cast, matching the weight-scale policy independently of provider defaults. The existing operand tests check this forwarding while still executing each real provider kernel.

Depends on pytorch/ao#4995 as the single AO dependency. That PR preserves Protonu's kernel from #4895 and includes current nonfinite handling, the 32x1 FakeTensor fix (also addressed by Aleksandar in #4702), and byte-oracle regressions. The original PRs are provenance, not additional dependencies.

## Test Plan

Real CUDA comparisons validate all cast directions and two backward contributions, including in-place accumulation. Real two-rank FSDP checks cover compiled RAF=true, retained PP weights and CUDA-graph replay. Schema/FakeTensor opchecks validate provider layouts. Matched CUDA-graph replay checks defined output bytes, timing and actual target kernels.

```bash
python -m pytest tests/unit_tests/cpu/quantization/test_quantization.py tests/unit_tests/cpu/quantization/test_quantized_fsdp_tensor.py -q
python -m pytest tests/unit_tests/gpu/quantization/test_mxfp8_linear.py -q
python -m pytest tests/unit_tests/gpu/quantization/test_mxfp8_fsdp.py -q -k 'cutedsl or (tensor_lifecycle and not simple)'
pre-commit run --all-files
```
sanketpurandare added a commit to pytorch/torchtitan that referenced this pull request Oct 13, 2026
## Agent details

Prepared with AI assistance.

Reuse TorchAO's fused rowwise/columnwise activation cast and existing single-axis CuTeDSL casts, consuming blocked E8M0 scales without separate rearrangement kernels. Keep CUDA as the default and expose CuTeDSL as an explicit activation_quantization_backend option through both the linear and its converter. Input-save format, weight quantization, output ownership, FSDP lifetime and gradient accumulation policy are unchanged.

CuTeDSL requires 128-aligned BF16 matrices and the required runtime/API at construction. Unsupported shapes fail explicitly rather than padding activations to a larger GEMM or silently selecting another implementation. Represent dense matrices as one group, view the valid column-scale prefix, and retain immutable real endpoints for CUDA-graph pointer lifetime. Fake endpoints never enter the real cache.

Explicitly request RCEIL for every activation cast, matching the weight-scale policy independently of provider defaults. The existing operand tests check this forwarding while still executing each real provider kernel.

Depends on pytorch/ao#4995 as the single AO dependency. That PR preserves Protonu's kernel from #4895 and includes current nonfinite handling, the 32x1 FakeTensor fix (also addressed by Aleksandar in #4702), and byte-oracle regressions. The original PRs are provenance, not additional dependencies.

## Test Plan

Real CUDA comparisons validate all cast directions and two backward contributions, including in-place accumulation. Real two-rank FSDP checks cover compiled RAF=true, retained PP weights and CUDA-graph replay. Schema/FakeTensor opchecks validate provider layouts. Matched CUDA-graph replay checks defined output bytes, timing and actual target kernels.

```bash
python -m pytest tests/unit_tests/cpu/quantization/test_quantization.py tests/unit_tests/cpu/quantization/test_quantized_fsdp_tensor.py -q
python -m pytest tests/unit_tests/gpu/quantization/test_mxfp8_linear.py -q
python -m pytest tests/unit_tests/gpu/quantization/test_mxfp8_fsdp.py -q -k 'cutedsl or (tensor_lifecycle and not simple)'
pre-commit run --all-files
```
@sanketpurandare
sanketpurandare marked this pull request as ready for review October 13, 2026 02:36
@pytest.mark.skipif(
not _mxfp8_cutedsl_kernels_available, reason="MXFP8 cutedsl kernels not available"
)
def test_mxfp8_quantize_2d_1x32_32x1_cutedsl_input_validation():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i'd remove this, low value test

@vkuzo

vkuzo commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

@claude review, pay special attention to how the new kernel is tested vs reference

@claude

claude Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Claude finished @vkuzo's task in 2m 59s —— View job


Review of PR #4995: fused dual-axis MXFP8 quantization

  • Gather context (diff, kernel, wrapper, tests)
  • Analyze how the fused kernel is tested vs the reference
  • Review kernel / wrapper / FakeTensor changes
  • Post review

I reviewed this by reading the code only; I didn't run anything on GPU (the runner has no SM10x). I traced the kernel's indexing by hand: the rowwise and colwise scale swizzles (sR8/sS8), the prmt de-interleave for q_col, the s_row tile ordering, the K-groups slab placement, the uncovered-tail zeroing and the over-allocation clear. On both the 256- and 512-thread paths, all of it matches to_blocked / triton_mx_block_rearrange_2d_K_groups. The 32x1 FakeTensor fix ((1, m) strides and (k, m_blocks) for non-blocked scales) matches what cutedsl_quantize_2d_32x1.py:941-968 actually allocates.


Testing vs reference (the main focus)

1. Nothing compares the kernel to to_mx, and the stated reason looks out of date.
Every numerics test compares bytes against the old three-kernel path (1x32 cutedsl + CUDA dim1 + triton K-groups swizzle). If one of those kernels is wrong, this one is wrong in the same way and the tests still pass. The header comment (test_kernels.py, around line 1694) gives the reason:

the fused kernel is deliberately bug-compatible with the colwise CUDA kernel's degenerate-block convention, so a to_mx reference would disagree

On current main I don't think that's true any more:

  • _to_mx_rceil computes the reciprocal with _e8m0_scale_to_reciprocal_fp32, which is 254 - scale_biased. For scale byte 0 that gives 2^127, the same multiplier the kernel uses.
  • With NOSAT for Inf/NaN (byte 255, and 254-255 wraps to 255, so the data is NaN), the two should agree there as well.

Suggestion: add a direct to_mx(..., ScaleCalculationMode.RCEIL) comparison for both axes. Compare q_row/q_col bytewise, and compare scales after from_blocked (rowwise) or a per-group un-swizzle (colwise). Run it on randn, on the degenerate-scale cases and on the mixed-magnitude case. If to_mx really does still disagree somewhere, mark that case xfail with the reason, so the difference is written down and tracked rather than hidden behind a reference that matches the kernel's quirks. Then either update the header comment or delete it.

2. Special values are only checked against the shared contract for the rowwise half.
test_..._special_value_semantics puts one case per row, so only q_row/s_row go through assert_mxfp8_semantics. The colwise half only gets compared to the CUDA dim1 kernel. A transposed tile would cover it at almost no cost: x[:32, :num_cases] = cases.inputs.T, repeated down the four 32-row blocks. Then run assert_mxfp8_semantics on q_col[:num_cases, ...] together with the un-swizzled s_col.

3. Empty groups are never tested.
generate_jagged_offs samples without replacement, and _fused_1x32_32x1_offs_with_tail asserts n_blocks >= num_groups, so no group in any test has size 0. Empty experts are common in MoE. They also go straight through the group-search loop in _kernel (quant.py-style scan around cutedsl_quantize_2d_1x32_32x1_fused.py:440), which is where the TorchTitan grad_norm=inf bug came from. Please add explicit cases:

  • offs=[0, 256, 256, 512] (leading and middle empty groups)
  • offs=[512, 512] (trailing empty group)
  • offs=[0] with total_M=256 (everything uncovered)

4. The flat-prefix s_col comparison is correct, with one gap.
Comparing exp.shape[0] * (cols - 4G) flat bytes is right. Group sizes are multiples of 128, so group_cols % 4 == 0, no group needs padding, and the consumed region is exactly the first N * total_M/32 bytes. A short comment saying this would help the next reader. The gap: only test_..._dirty_output_buffers checks the slack bytes past that region. The extra uncovered == 0 assertion in the tail test duplicates what the prefix comparison already checks, since the reference is new_zeros.

5. test_..._determinism: I agree it adds little.
A race between CTAs would make the bytewise reference comparison fail on almost every run, and the dirty-buffer test would catch any bytes left unwritten. I'd drop it along with the input-validation test @vkuzo flagged.

6. Small test-hygiene points

  • generate_jagged_offs uses random.sample with no seed, so the group boundaries change from run to run. If anything fails it will be hard to reproduce.
  • The undo_scale lambda reformat in test_cuda_mx_3d_cutedsl_numerics is unrelated churn.

Kernel / wrapper

7. Dead code: the if mt >= mblocks: branch (cutedsl_quantize_2d_1x32_32x1_fused.py:268-278).
The grid is [N//TN, M//TM, 1], so mt < mblocks always holds and this branch never runs. The over-allocation slab is actually cleared by the mt == 0 loop at lines 478-485, so the comment "One extra CTA per group…" describes something that doesn't happen. Remove the branch and un-indent the else: body.

8. _scale's e == 0 special case does nothing (lines 211-212).
When e == 0 the general formula already gives (254-0)<<23 = 0x7F000000. That is exactly the degenerate_ib passed at every call site. The bits == 0 sub-case only fires when every element is ±0, and ±0 times anything is still ±0. So you can drop both the if and the degenerate_ib parameter without changing behavior. This is worth doing because the parameter suggests a second convention exists when it doesn't.

9. The *_out buffers aren't validated (quantize_fused, lines 633-648).
_launch assumes contiguous buffers with exact shapes. run caches the compiled function keyed only on (x.shape, offs.shape), and it's compiled with assumed_align=16. A wrong-sized, strided or misaligned s_col_out would either write out of bounds or reuse a function compiled for a different dtype. Please assert shape, dtype (uint8/e4m3), is_contiguous() and data_ptr() % 16 == 0 for any buffer the caller passes in.

10. Offsets that aren't ascending get through.
validate_group_sizes only checks size % 128 == 0. A negative size like -128 passes, and then the group-search loop can make two tiles write to the same slab. The existing kernels have the same gap, but this kernel's placement math relies more heavily on offs being ascending, so adding curr_end >= prev_end to the device-side assert would be cheap insurance.

11. Minor

  • run launches on torch.cuda.current_stream() rather than the stream for x.device. That's wrong if the current device isn't x's device. Other cutedsl kernels have the same pattern, so this is low priority.
  • Nothing calls the new op yet; it isn't wired into the grouped-GEMM backward. That's fine for a kernel-only PR, but the description should say so.
  • No register_sharding for the new op, unlike the 1x32 and 32x1 ops. Fine to defer.

Summary

The kernel looks correct everywhere I traced it, and the destination-buffer, uncovered-tail and NaN/Inf tests are good additions. My main request, given the question asked: anchor the tests to to_mx directly instead of only to the three-kernel path, because the stated reason for avoiding to_mx seems out of date. Alongside that: add colwise special-value checks, empty-group cases and validation of the *_out buffers, and remove the unreachable mt >= mblocks branch.

Fix the dead branch and _scale → · Add to_mx / empty-group tests →

"torchao::mxfp8_quantize_2d_1x32_32x1_cutedsl", mutates_args=()
)
def _mxfp8_quantize_2d_1x32_32x1_cutedsl_custom_op(
x: torch.Tensor,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we match the argument order of surrounding APIs like _mxfp8_quantize_2d_1x32_cutedsl_custom_op

    x: torch.Tensor,
    block_size: int = 32,
    scaling_mode: str = "rceil",
    offs: torch.Tensor = None,

then assert for any unsupported values

@sanketpurandare sanketpurandare Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matched the public, custom-op and fake signatures to (x, block_size=32, scaling_mode="rceil", offs=None) and added consistent unsupported-value checks. offs=None selects dense placement in the same kernel without offset processing or scale slack; tensor offsets retain grouped, empty-expert and uncovered-tail behavior. Updated all AO callers and TT#5334's consumer, removing its synthetic endpoint/cache. Real/fake opchecks, actual Inductor compilation and the two-rank consumer checks passed.

@vkuzo

vkuzo commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

lgtm, can we just update the public arg order (see comment inline) and remove the unnecessary test

Comment on lines +1745 to +1748
q_row, s_row = mxfp8_quantize_2d_1x32_cutedsl(
x, block_size=32, scaling_mode="rceil", offs=offs
)
mx = _to_mxfp8_dim1_kernel_wrapper(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional nit: the authorative numerics reference is

, but this is also fine

@sanketpurandare sanketpurandare Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched the primary numerical checks to direct to_mx(RCEIL) for both axes, with per-group unblocking. This exposed a real scale0/1 cutoff bug: the fused kernel used 0x04E00000 instead of 0x04600000 (448*2^-127). Corrected it and added exact neighboring values on both CTA paths, mixed magnitudes, independent columnwise semantic cases and explicit empty groups. Finite bytes are checked exactly; FP8 NaN signs follow the shared semantic contract. The current 63-test suite and strict API documentation build passed.

Performnace of this fused cute-dsl quantizer is better than cuda, a single API for dense and grouped mxfp8 linears, no need for scale rearrangements since swizzled layouts are directly output.

AI-assisted implementation details:

> Keep Protonu's original authored fused kernel from #4895 as the
> parent commit and consolidate current-main correctness fixes in this PR.
> Credit: Protonu (@protonu), original commit
> 1d4e8d3; Aleksandar Samardzic's #4702
> independently addressed the single-axis FakeTensor metadata issue.
>
> Match the public/custom-op/fake argument order to surrounding APIs. None
> offsets select dense placement in the same kernel, with compact blocked
> scales and no synthetic endpoint or offset search. Tensor offsets retain
> grouped placement, empty experts and uncovered-tail behavior.
>
> Correct the scale0/1 RCEIL cutoff to 448*2^-127, preserve NOSAT nonfinite
> semantics, and compare both axes directly to to_mx with group unblocking.
> Cover exact cutoff neighbors, mixed magnitudes, special values, empty groups,
> dirty destinations, actual compile/fake contracts and device selection.
>
> Validate aligned destination metadata and nonoverlapping byte intervals.
> Enable device assertions explicitly for grouped compilation; CuTeDSL strips
> them by default. Reuse the endpoint validation loop for ordering and bounds.
> Remove unreachable CTA work and redundant reciprocal conventions.
>
> Test plan: affected fused/single-axis CUDA tests, repository Ruff checks,
> strict API documentation rendering, matched replay timings and traces, and
> real TorchTitan FSDP/MXFP8 eager/capture consumer tests.

Pull Request resolved: #4995
@sanketpurandare
sanketpurandare force-pushed the fused-mxfp8-dual-axis-quantization branch from 5fb606c to 86f7e2a Compare October 14, 2026 22:01
sanketpurandare added a commit to pytorch/torchtitan that referenced this pull request Oct 14, 2026
Performnace of this fused cute-dsl quantizer is better than cuda, a single API for dense and grouped mxfp8 linears, no need for scale rearrangements since swizzled layouts are directly output.

AI-assisted implementation details:

> Reuse TorchAO's fused dense rowwise/columnwise activation cast and existing
> single-axis CuTeDSL casts, consuming directly blocked E8M0 scales. Keep CUDA
> as the default and CuTeDSL as explicit activation_quantization_backend opt-in
> through the Linear and converter. Input-save format, weight quantization,
> output ownership, FSDP lifetime and gradient accumulation policy are unchanged.
>
> Use the provider's offs=None dense specialization, with compact column scales
> and no synthetic endpoint, cache, FakeTensor endpoint branch or padding trim.
> Construction rejects an older required-offset provider as well as missing
> symbols/runtime. Both real and fake paths use the same dense contract.
>
> CuTeDSL requires 128-aligned BF16 matrices. Unsupported shapes fail explicitly,
> without padding to a larger GEMM or silently changing implementations. Pin
> RCEIL for every activation cast to match the 32x32 weight-scale policy.
>
> Depends only on pytorch/ao#4995. Preserve credit to Protonu's original #4895
> kernel and Aleksandar's #4702 single-axis metadata work; those PRs remain
> provenance, not additional dependencies.
>
> Test plan: current CPU contracts, real CUDA cast directions/backward formats
> and accumulation, two-rank compiled/retained/CUDA-graph FSDP lifecycle,
> strict configuration/README rendering, complete repository lint and matched
> replay timings with actual target-kernel traces.
@sanketpurandare

Copy link
Copy Markdown
Contributor Author

Performnace of this fused cute-dsl quantizer is better than cuda, a single API for dense and grouped mxfp8 linears, no need for scale rearrangements since swizzled layouts are directly output.

AI-assisted implementation details:

Removed the separate input-validation test and consolidated repeated-write coverage into the dirty-buffer test. Kept focused checks for the new real/fake API and destination contracts. Updated the original AO#4995 and TT#5334 owners, with no new PRs. The current AO suite passed 63 tests; the exact TT consumer passed 69 CPU tests, 32 Linear GPU tests and 6 real two-rank FSDP checks.

@sanketpurandare
sanketpurandare requested a review from vkuzo October 14, 2026 23:22
@vkuzo

vkuzo commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Claude finished @vkuzo's task in 3m 1s —— View job


Re-review of PR #4995 (after 86f7e2a)

  • Gather context (latest diff)
  • Review kernel (_scale cutoff, placement, validation) and wrapper / fake
  • Review tests vs to_mx reference
  • Check prior review items and @vkuzo's requests
  • Post review

I reviewed by reading the code only. I couldn't run anything: the runner has no SM10x, and ad-hoc scripts weren't permitted here.


@vkuzo's requests: both done

  • Argument order. The public wrapper, custom op and fake now all take (x, block_size=32, scaling_mode="rceil", offs=None). Unsupported values are asserted in both quantize_fused and the fake. test_..._real_fake_api_contract checks real and fake reject them the same way.
  • Low-value tests. The input-validation test and the standalone determinism test are gone. Repeated launches into dirty buffers are now folded into test_..._dirty_output_buffers.

Prior Claude review items

# Item Status
1 Anchor tests to to_mx ✅ _assert_fused_1x32_32x1_matches_reference now uses to_mx(RCEIL) on both axes, with per-group from_blocked for s_col
2 Colwise special values ✅ special_value_semantics[colwise] uses a transposed case tile
3 Empty groups ✅ [0,256,256,512], [512,512], [0], []
6 Unseeded offsets ✅ deterministic offsets and a local torch.Generator
7 Dead mt >= mblocks branch ✅ removed; the slack is cleared by the mt == 0 CTAs
8 _scale e == 0 / degenerate_ib ✅ removed
9 *_out validation ✅ shape, dtype, device, contiguity, 16B alignment and byte-interval overlap
10 Ascending offsets ✅ nonnegative, nondecreasing and <= total_M device asserts
11 Stream for x.device ✅ current_stream(x.device), with a 2-GPU test

Switching to to_mx immediately found a real bug: the old cutoff was 0x04E00000 instead of 0x04600000. That's a good sign the reference change was worth it.


Checking _scale against to_mx

cutedsl_quantize_2d_1x32_32x1_fused.py:208-225

I traced the integer RCEIL by hand:

  • (bits + 0x1FFFFF) >> 23 rounds the exponent up exactly when the mantissa is above 0.75. The threshold is 0.75 because 448 = 1.75·2⁸.
  • For a mantissa of exactly 1.75·2^E it gives E-8 (biased), which is exactly 448·2^(E-8). That's correct.

to_mx computes fl(amax · fl(1/448)) and then rounds up. I checked whether that floating-point step can land on a different power of two:

  • fl(1/448) rounds up. But 7·fl(1/7) is within 2⁻²⁴ of 1, so 1.75·2^E·fl(1/448) still rounds to exactly 2^(E-8).
  • The nearest bf16 value below a boundary is about 2⁻⁸ away in relative terms, far more than fp32 rounding error.
  • The only fp32-subnormal descales that matter sit next to 2⁻¹²⁷, where precision is still about 2⁻²² relative.

So the new formula should match to_mx for every finite bf16 amax. This also explains why the old cutoff got through: the 1e-38 / 1e-30 degenerate tests land on either side of the [0x0460, 0x04E0) window, never inside it.

Suggestion (cheap, high value): replace or extend the 6-value scale_zero_boundary test with an exhaustive sweep of every positive finite bf16 magnitude. That is 0x0000–0x7F7F, about 32.6K values, one value per row in an (32768, 128) input for rowwise, and the same tile transposed for colwise. It's a single 8 MB launch, compared byte for byte against to_mx, and it would close this whole class of bug for good. Today only the 0x0460 and 0x04E0 neighbourhoods are pinned, so any other off-by-one exponent region would depend on randn happening to hit it.


New observations

1. The quant.py docstring still describes the composition as the contract. In mxfp8_quantize_2d_1x32_32x1_cutedsl:

Bit-exact with mxfp8_quantize_2d_1x32_cutedsl composed with the colwise cast and a standalone K-groups swizzle, degenerate blocks included

The degenerate, boundary, mixed-magnitude and special-value tests now check only against to_mx. Only the randn numerics test still compares to the composition. Please reword this to "bit-exact with to_mx(..., RCEIL) on both axes (FP8 NaN sign aside)". As written, it promises something the tests no longer check.

2. The shared validate_group_sizes change also affects the existing 1x32 and 32x1 kernels (cute_utils.py:343). Both already compile with --enable-assertions when offs is given (cutedsl_quantize_2d_1x32.py:854-858). So they now also reject negative or descending offsets, though not max_offset, since they pass none. I think that's the right behaviour. It is a behaviour change outside the new kernel, though, so please mention it in the PR description. It would also be worth running the existing 1x32/32x1 grouped tests to confirm nothing feeds them non-monotonic offs.

3. Grouped-mode cost grows linearly with G in every CTA; worth benchmarking at realistic G. In _kernel:

  • thread 0 of every CTA runs validate_group_sizes, a serial loop of G loads (line 249);
  • 128 threads then each re-scan offs with a fully unrolled range_constexpr(groups) that never exits early (lines 433-440).

At G = 8 this doesn't matter. At G = 64–256 (local experts under low EP) it costs per-CTA latency and also bloats compile size and compile time. The timings in the PR description are for dense mode only. The checked-in benchmark covers grouped mode, but the default configs top out at small G. Could you add a large-G row, e.g. (16384, 7168, 128)? If the cost shows up, there are cheap fixes:

  • validate only in CTA (0,0);
  • scan once per CTA into SMEM, or use a dynamic loop with break.

4. The checked-in benchmark doesn't reproduce the headline table. The PR description compares dense CuTeDSL against "dense CUDA + two scale rearrangements". bench_cutedsl_quantize_2d_1x32_32x1_fused.py only compares grouped fused against the grouped three-kernel composition. Please either add the dense comparison to the script or say in the description where those numbers came from.

5. test_..._offset_bounds is fragile. The subprocess runs from test.prototype.moe_training.test_kernels import ... with no cwd= or PYTHONPATH, so it only works when pytest is launched from the repo root. Pass cwd set to the repo root (derived from __file__), or move _run_mxfp8_fused_offset_validation into a small standalone module or script string. Each of the four parametrizations also pays a fresh Python + CuTeDSL import and JIT compile. That's acceptable, but if CI time matters you could collapse the three failure cases into one parametrization.

6. Minor

  • _launch's mCS layout length is N * (M//32 + G*4) // 4. That assumes ceil_div(N,128)*128 == N, which holds only because N % 128 == 0 is asserted. s_col_shape and the fake still carry the ceil_div. Either drop it everywhere or add a short comment, so nobody relaxes the N constraint later and gets an out-of-bounds write.
  • _aligned16(offs) quietly clones a misaligned offs. That's fine, but x and the outputs assert alignment instead. A one-line comment on why offs gets a clone rather than an assert would help.
  • register_sharding is still missing for the new op. Fine to defer, but worth a TODO since the 1x32 and 32x1 ops have one.

Summary

Both of @vkuzo's requests are done, and all the major points from the previous review are fixed. The tests are now anchored to to_mx, and that change found and fixed a real cutoff bug. By my hand analysis the corrected _scale should agree with to_mx for every finite bf16 amax.

Remaining asks, in priority order:

  1. An exhaustive bf16-magnitude sweep test, which would turn that analysis into a check.
  2. Fix the stale "bit-exact with composition" docstring.
  3. A grouped benchmark at large G, given the per-CTA validation and scan.
  4. Note the shared validate_group_sizes behaviour change in the PR description.

None of these block correctness.

Add exhaustive bf16 sweep test → · Fix docstring + test cwd →

@vkuzo vkuzo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing! thank you @sanketpurandare for making this ready, and also thank you @protonu for the great work in #4895!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: training quantize_ api training flow

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants