Skip to content

Fix: fused QUproj + RoPE + Quant - #3421

Open
ghadiaravi13 wants to merge 3 commits into
NVIDIA:mainfrom
ghadiaravi13:rghadia/fused_mlperf_final
Open

Fix: fused QUproj + RoPE + Quant#3421
ghadiaravi13 wants to merge 3 commits into
NVIDIA:mainfrom
ghadiaravi13:rghadia/fused_mlperf_final

Conversation

@ghadiaravi13

Copy link
Copy Markdown

Description

Modify the FusedMLAQUpProjFunction to absorb norm inside the autgrad function, so that the fp32 output from the normalization is directly quantized to MXFP8 without intermediate bf16 rounding.

Fixes # (issue)

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Change A
  • Change B

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

FusedMLAQUpProjFunction now owns the normalization instead of consuming a
pre-normalized BF16 activation. TELayerNormColumnParallelLinear hands its input
quantizer to the norm kernel, so MXFP8 is produced in one step from the FP32
accumulator; splitting that into a standalone norm plus a separate quantize
rounds twice, which under NVTE_NORM_FWD_USE_CUDNN=1 moves ~3.3% of the E4M3
codes and perturbs the rsigma the norm backward consumes.

FusedMLAQUpProjRopeQuant.run therefore takes an already-quantized input on the
FP8 path rather than quantizing again, and backward gained the RMSNorm bwd.
Scope is RMSNorm at TP=1; both are asserted.

Replaces the env-gated FusedMLAQUpProjNormFunction that carried this on
cont-fused: there is now one path, not two.
Updated comments in FusedMLAQUpProjRopeQuant and FusedMLAQUpProjFunction. Streamlined the description of the fused Q up-projection process.

Signed-off-by: Ravi Ghadia <rghadia@nvidia.com>
@github-actions github-actions Bot added the community-contribution PRs from external contributor outside the core maintainers, representing community-driven work. label Aug 24, 2026
@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR moves RMSNorm inside the fused MLA Q up-projection autograd function so its FP32 accumulator can be quantized directly to MXFP8 before fused GEMM, RoPE, and output quantization.

  • Adds an unfused custom backward covering inverse RoPE, projection gradients, and RMSNorm gradients.
  • Reuses the normalization-produced MXFP8 activation for forward GEMM and weight-gradient computation.
  • Exposes FusedMLAQUpProjFunction through the attention and root PyTorch namespaces.
  • Restricts this temporary fused path to RMSNorm and single-rank tensor parallelism.

Confidence Score: 4/5

The PR should not merge until the RoPE backward launch validates that the rotary interval fits within each query head; the missing API documentation is non-blocking.

Caller-controlled head metadata can make the new Triton backward kernel cross head boundaries and access beyond the gradient allocation because its mask bounds only the starting offset.

Files Needing Attention: transformer_engine/pytorch/attention/fused_mla_q_uproj.py and transformer_engine/pytorch/init.py

Important Files Changed

Filename Overview
transformer_engine/pytorch/attention/fused_mla_q_uproj.py Adds normalization and complete custom autograd around fused MLA projection, but backward lacks a required head-dimension bound before launching the Triton RoPE kernel.
transformer_engine/pytorch/attention/init.py Publicly exports the new autograd function; this makes its low-level caller-controlled dimension contract directly reachable.
transformer_engine/pytorch/init.py Re-exports the new function at the root PyTorch API without adding the corresponding API documentation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  X[Pre-normalization input] --> N[RMSNorm]
  N --> Q[Direct MXFP8 quantization]
  Q --> G[Fused Q up-projection GEMM]
  G --> R[Per-head RoPE]
  R --> O[MXFP8 query]
  O --> RB[Inverse RoPE backward]
  RB --> LB[Linear backward]
  LB --> NB[RMSNorm backward]
Loading

Reviews (1): Last reviewed commit: "refactor: simplify comments and document..." | Re-trigger Greptile

Comment on lines +461 to +475
rotary_bwd_q_kernel[grid](
dq3,
cos.contiguous(),
sin.contiguous(),
qk_head_dim,
qk_pos_emb_head_dim,
nh,
1,
None,
None,
dq3.stride(0),
dq3.stride(1),
0,
1,
)

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.

P1 Unbounded per-head RoPE access

When q_head_dim matches the fused output width but qk_head_dim + qk_pos_emb_head_dim exceeds it, the backward kernel's mask bounds only each head's starting offset, so its loads and stores cross into adjacent heads and beyond the final head, causing corrupted gradients or a CUDA illegal-memory-access failure.

Knowledge Base Used: PyTorch attention execution

from transformer_engine.pytorch.module import UserBufferQuantizationMode
from transformer_engine.pytorch.attention import DotProductAttention
from transformer_engine.pytorch.attention import FusedMLAQUpProjRopeQuant
from transformer_engine.pytorch.attention import FusedMLAQUpProjFunction, FusedMLAQUpProjRopeQuant

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.

P2 Undocumented public autograd API

FusedMLAQUpProjFunction is now exported from the root and attention namespaces without a corresponding entry in docs/api/pytorch.rst, leaving its extensive argument and shape contract absent from the generated public API reference.

Knowledge Base Used: PyTorch runtime and public API

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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

Labels

community-contribution PRs from external contributor outside the core maintainers, representing community-driven work.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant