Applied review comments 1, 2, 3, 4, 6, 7, 8 from PR #667 in one pass. Comments 5 and 9 will be answered on-thread (see below). Applied ======= #7 cpp_extensions/gemm.py: gate NVTE_USE_GEMM_TRITON behind IS_HIP_EXTENSION. Our Triton kernels use gfx942/gfx950-specific MFMA instructions and autotune configs; refuse to enable on non-HIP builds. #1 + #4 + #8: deduplicate dtype utilities against triton_kernels/common.py, which is the authoritative source shared across all Triton kernel backends here. - gemm_common.py: drop our copies of _get_fp8_dtypes(), torch_to_te_dtype(), te_to_torch_dtype(). Keep is_fp8_dtype() (operates on tex.DType; common.py's is_fp8_torch_dtype operates on torch dtypes -- different signatures), reinterpret_as_fp8_tensor, getGemmOutputShape, product. - gemm_wrapper.py: import torch_dtype_to_te_dtype / te_dtype_to_torch_dtype from ..common; use those instead of the removed local names. - __init__.py: drop the redundant re-exports (torch_to_te_dtype, te_to_torch_dtype, _get_fp8_dtypes). Note in the file directing callers to triton_kernels.common. - test_gemm_kernel.py: import get_torch_e4m3_type, get_torch_e5m2_type, torch_dtype_to_te_dtype from triton_kernels.common; define a local _get_fp8_dtypes() shim over them to keep the test's parametrization tables unchanged. - test_gemm_mxfp8.py: previously used _get_e4m3_dtype() -- dropped along with the test that used it (see below). #2 + #3: add module docstrings to test_gemm.py, test_gemm_kernel.py, test_gemm_mxfp8.py making the scope of each file explicit and pointing at the sibling files. Prompted by wangye805's confusion between test_gemm.py and test_gemm_kernel.py, and by the question of whether MXFP8 is only tested in test_gemm_mxfp8.py. #6 test_numerics.py: rewrite the comment above and the reason= text for _skip_grouped_under_gemm_triton to make it explicit that the skip is a *backend mismatch* between the two sides of the comparison, not Triton non-determinism. - sequential side -> our Triton (via NVTE_USE_GEMM_TRITON=1) - grouped side -> hipBLASLt / CUTLASS / AITER-Triton grouped (controlled by the SEPARATE NVTE_USE_GROUPED_GEMM_TRITON env var) With just NVTE_USE_GEMM_TRITON=1 set, the two sides diverge in fp32 rounding because they run different kernels. Not our Triton being non-deterministic. Also removed test_mxfp8_kernel_with_simulated_data (an early kernel- bring-up smoke test that only asserted non-zero output; MXFP8 correctness is fully covered by test_gemm.py::test_triton_vs_*_mxfp8 with real MXFP8Tensor and both PyTorch/C++ references). To be answered on-thread ======================== #5 (conftest.py hook vs explicit skip lists) -- keep the hook: it fires only on three specific ValueError substrings from our own gate code, is self-retiring when the gates are relaxed, and explicit skips would require ~1300 marks. Detail in the PR reply. #9 (Float8TensorWrapper / MXFP8TensorWrapper vs the TE tensor types) -- defer; needs a side-by-side to see how much of the wrapper API is genuinely required for the Triton-kernel call surface (fields accessed, dimension reordering, missing-transpose fallbacks) vs. what could route through the TE tensor types directly. Verified on gfx950 / PyTorch 2.10 -- all three direct suites clean: test_gemm.py 212 pass / 72 skip test_gemm_kernel.py 450 pass / 606 skip test_gemm_mxfp8.py 2 pass (was 3; smoke test removed)

Uh oh!

F8: Interop investigation considering G's discussion and feedback #6

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions