Improve device-init grouped linear module with single grouped weight support #3224
Improve device-init grouped linear module with single grouped weight support #3224zhongbozhu wants to merge 9 commits into
Conversation
aa3b9d1 to
47ba66a
Compare
ff7eee2 to
a43f70f
Compare
33a79ed to
6defdba
Compare
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
for more information, see https://pre-commit.ci
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
6536aa1 to
1169d6e
Compare
for more information, see https://pre-commit.ci
| is_grad_enabled = torch.is_grad_enabled() | ||
| num_gemms = self.num_gemms | ||
|
|
||
| if FP8GlobalStateManager.fp8_graph_capturing(): |
There was a problem hiding this comment.
Note: this code block was deleted because it was duplicated
|
/te-ci pytorch |
Greptile SummaryThis PR improves the device-init grouped linear module by adding full support for a single grouped weight/bias parameter (
Confidence Score: 4/5The changes are well-structured and covered by targeted tests; the main risks are the unguarded quantizer-internal flag mutation and test helpers that would silently skip on future GPU generations. The core forward/backward plumbing for single grouped parameters is logically consistent and the new tests exercise the key paths (BF16, MXFP8, fuse_wgrad, delay_wgrad, return_bias). The weight_quantizer.internal flag is mutated without a try/finally guard — an exception during quantization would leave it in the wrong state for all subsequent calls. The device-capability upper bound <= (11, 0) in both test helpers is tighter than the production gating, meaning tests would silently skip on SM >= 12.x hardware. The CUDA kernel integer division first_logical_dim / num_tensors lacks an assertion, though the invariant is enforced upstream. transformer_engine/pytorch/module/grouped_linear.py (quantizer-internal exception safety) and the two test helper functions in test_grouped_linear.py and test_grouped_mlp.py (device capability upper bound). Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant GroupedLinear_Module
participant _GroupedLinear_Autograd
participant PrepareWeights
participant tex_group_quantize
participant GroupedGEMM
Caller->>GroupedLinear_Module: forward(x, m_splits, is_first_microbatch)
GroupedLinear_Module->>GroupedLinear_Module: resolve grouped_gemm_backend
GroupedLinear_Module->>GroupedLinear_Module: get weight/bias tensors (single or discrete)
GroupedLinear_Module->>_GroupedLinear_Autograd: "apply(inp, m_splits, non_tensor_args, *weights, *biases)"
_GroupedLinear_Autograd->>PrepareWeights: "_prepare_weights_for_grouped_tensor_gemm(single_grouped_weight=True)"
alt BF16 weight
PrepareWeights-->>_GroupedLinear_Autograd: GroupedTensorStorage (view)
else MXFP8 weight, workspace cached
PrepareWeights->>tex_group_quantize: "group_quantize(src, quantizer, N, noop_flag, output=workspace)"
tex_group_quantize-->>PrepareWeights: workspace (in-place, same pointers)
PrepareWeights-->>_GroupedLinear_Autograd: cached GroupedTensorStorage
else MXFP8 weight, no workspace
PrepareWeights->>tex_group_quantize: group_quantize(src, quantizer, N)
tex_group_quantize-->>PrepareWeights: new GroupedTensorStorage
PrepareWeights-->>_GroupedLinear_Autograd: new workspace (saved to _fp8_workspaces)
end
_GroupedLinear_Autograd->>GroupedGEMM: general_grouped_gemm_for_grouped_tensor(weight, x, out)
GroupedGEMM-->>_GroupedLinear_Autograd: output tensor
_GroupedLinear_Autograd-->>GroupedLinear_Module: (output, new_workspaces)
GroupedLinear_Module->>GroupedLinear_Module: update _fp8_workspaces["weight"]
alt return_bias and single_grouped_bias
GroupedLinear_Module-->>Caller: (output, bias_GroupedTensor)
else
GroupedLinear_Module-->>Caller: output
end
|
| def _require_native_grouped_tensor_gemm(*, mxfp8: bool = False) -> None: | ||
| """Skip unless the native GroupedTensor grouped GEMM is available.""" | ||
| device_capability = torch.cuda.get_device_capability() | ||
| if not (9, 0) <= device_capability <= (11, 0): | ||
| pytest.skip("Native GroupedTensor grouped GEMM requires Hopper or Blackwell.") |
There was a problem hiding this comment.
Upper-bound capability check blocks future architectures. The
<= (11, 0) cap means any GPU with SM ≥ 12.x will be unconditionally skipped even if it supports the native grouped-tensor GEMM. The production path in _is_grouped_tensor_path_supported checks (9, 0) <= capability with no upper bound, so the test guard is more restrictive than the implementation. Consider removing or raising the upper bound.
| def _require_native_grouped_tensor_gemm(*, mxfp8: bool = False) -> None: | |
| """Skip unless the native GroupedTensor grouped GEMM is available.""" | |
| device_capability = torch.cuda.get_device_capability() | |
| if not (9, 0) <= device_capability <= (11, 0): | |
| pytest.skip("Native GroupedTensor grouped GEMM requires Hopper or Blackwell.") | |
| def _require_native_grouped_tensor_gemm(*, mxfp8: bool = False) -> None: | |
| """Skip unless the native GroupedTensor grouped GEMM is available.""" | |
| device_capability = torch.cuda.get_device_capability() | |
| if not device_capability >= (9, 0): | |
| pytest.skip("Native GroupedTensor grouped GEMM requires Hopper or newer.") |
| compute_capability = get_device_compute_capability() | ||
| if not (9, 0) <= compute_capability <= (11, 0): | ||
| return False |
There was a problem hiding this comment.
Same upper-bound issue as in
test_grouped_linear.py. The check (9, 0) <= compute_capability <= (11, 0) will return False for any SM ≥ 12.0, causing single_grouped_weight and single_grouped_bias test cases to be silently skipped on future GPU generations even if the backend supports them.
| compute_capability = get_device_compute_capability() | |
| if not (9, 0) <= compute_capability <= (11, 0): | |
| return False | |
| compute_capability = get_device_compute_capability() | |
| if not compute_capability >= (9, 0): | |
| return False |
There was a problem hiding this comment.
The biggest change in this PR is that TE is abandoning any attempt to make single_grouped_weight=True a general feature. Things must be exactly right, or we crash. Given how delicate and experimental this feature has been, I'm not opposed.
The second change is that users must opt-in to access the grouped GEMM kernel. This is also reasonable, since it has alignment requirements for m_splits and it's helpful having a way for users to accept that stricter contract.
We are experiencing many test failures. Given that single_grouped_weight is no longer a general feature, I think it's reasonable we move the corresponding tests to test_grouped_linear.py and test_grouped_mlp.py.
| raise ValueError( | ||
| "The native grouped_tensor path requires CUDA m_splits. Pass a CUDA int64 " | ||
| "tensor, or select grouped_gemm_backend='legacy'." | ||
| ) |
There was a problem hiding this comment.
I get that the h2d memcpy is suboptimal, but it's trivially easy to handle. Erroring out seems excessively rigid.
| raise ValueError( | |
| "The native grouped_tensor path requires CUDA m_splits. Pass a CUDA int64 " | |
| "tensor, or select grouped_gemm_backend='legacy'." | |
| ) | |
| m_splits = m_splits.to(device=device) |
We need to handle the d2h case anyways when the user has specified grouped_gemm_backend="grouped_tensor", but it's not supported and we fallback to split-quantize.
| EXPERIMENTAL and subject to change. Gated by the | ||
| ``NVTE_GROUPED_LINEAR_SINGLE_PARAM`` environment variable: if the env var | ||
| is not set this argument is forced to ``False`` with a warning. | ||
| grouped_gemm_backend : {"legacy", "grouped_tensor"}, default = None |
There was a problem hiding this comment.
These are misleading names.
- Legacy implies we are deprecating it, but in fact it is the default backend and it is the fallback when the grouped tensor impl is not supported.
- As mentioned in https://github.com/NVIDIA/TransformerEngine/pull/3224/changes#r3618050729, it's confusing that we might not actually perform the backend specified by the user. The user might put
grouped_gemm_backend="grouped_tensor"and we'll actually do split-quantize because the cuBLAS version is old or something.
Really, this is not a backend config. It's a hint where the user promises to provide m_splits with a certain alignment. I'd suggest changing this to a bool like enable_grouped_tensor_backend. Alternatively if we want to generalize in case we add more backends in the future, we could have a string like enabled_backends (enabled_backends="grouped_tensor,future_backend").
Description
Please include a brief summary of the changes, relevant motivation and context.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: