Repository navigation
Note when fused SDPA kernels are available - #4863
Open
electron-rare wants to merge 1 commit into
Open
electron-rare wants to merge 1 commit into
electron-rare wants to merge 1 commit into
Conversation
On the Metal backend a fused kernel only exists for some input shapes. Say so in the scaled_dot_product_attention docstring, and that with force_fused=True the raised error gives the reason.
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.
I'm setting up benchmark for vision encoders—ViT models—on Apple Silicon. I had to read through the Metal code to figure out if the fused path was being used; it is not in the documentation.
Without this information, a user has no way of knowing whether their model benefits from the fused kernel, nor how to choose a shape that does benefit from it outside of the usual cases: the fallback is silent and the result is identical—only the speed changes (measured on ViT shapes : the fused kernel runs 15 to 87% faster on the M1 Max). A lightweight ViT with head dimension 48 never benefits from it, without warning.
Motivation: The docstring describes
force_fusedwithout mentioning that the fused kernel depends on the shape of the inputs. While setting up an SDPA benchmark for typical ViT shapes (L = 197 and 257, D = 64) on Metal, I wanted to know if the fused path was actually taken, and there’s nothing in the documentation that lets you know this without reading the Metal code or forcing it withforce_fused=True. To see what happens in an uncovered case, I tried D = 48:force_fused=Trueraises the error “no fused kernel is available: the full attention kernel supports head dims {64, 72, 80, 96, 128, 192, 256} …”, whereas D = 64 works. Small head dimensions exist in lightweight ViTs (D = 16 to 48), so this is not just a theoretical issue.I therefore propose adding a short note to the docstring of
mx.fast.scaled_dot_product_attention(python/src/fast.cpp) to state that, on the Metal backend, a fused kernel exists only for certain input shapes (head dimension, query length, mask type), that the unfused path is taken otherwise, and that, ifforce_fused=Trueis passed in this case, an error is raised with the reason.The note refers to the error message rather than listing the dimensions: the list depends on the query length, the mask, and the GPU, and would change with the code. The error is raised in
use_fallback(mlx/backend/metal/scaled_dot_product_attention.cpp), which uses the reason provided byhas_fused_kernel.I verified this with mlx 0.32.4 on M1 Max (macOS 27.0.1) and M3 Ultra (macOS 27.2):
force_fused=Truesucceeds for D = 64 (ViT shapes, L = 197 and 257) and raises the above message for D = 48; withoutforce_fused, D = 48 takes the unfused path and yields the same result as matmul + softmax.Docstring only: one file, +7 lines, no tests added. A separate addition to the
sdpa_bench.pybenchmark (ViT shapes) may follow in another PR, currently being prepared and reviewed.