Fix FP8 LayerNorm scale bug, benchmark byte-count formula, and missing flash_attn wrapper - #1
Open
pauliano22 wants to merge 1 commit into
Open
Conversation
…rapper
- kernels/layer_norm_fp8.py: the kernel only stored the per-row scale
for row 0 (`if tl.program_id(0) == 0`), leaving every other row's
entry in the `scales` tensor as uninitialized garbage from
torch.empty. Dequantizing any row but the first with `y_fp8 * scale`
produced wrong results. Verified via a CPU interpreter-mode
reproduction: the original code left 7/8 rows uninitialized; the fix
gives all rows a correct, distinct scale matching the fp32 reference
layernorm to within float error.
- benchmarks/bench_layernorm_fp8.py: the GB/s calculation always used
the BF16 byte-traffic formula (M*N*6) even for the FP8 line, which
the same comment says should be M*N*5 — inflating the reported FP8
throughput by ~20%. Now selects the multiplier per provider.
- kernels/flash_attn.py: `benchmarks/bench_flash_attn.py` imports
`flash_attn` from this module, but only the low-level
`flash_attn_kernel` existed — the benchmark has never been able to
run (ImportError), which is presumably why the README only reports
a PyTorch baseline for this section and no Triton number. Added the
host-side `flash_attn(q, k, v)` wrapper mirroring the pattern used
in the other kernel files. While wiring it up, found and fixed two
more issues that would have made the kernel fail to even compile:
- `d_head` was used as a `tl.zeros`/`tl.arange` shape argument but
was never marked `tl.constexpr`, which Triton requires for
compile-time shapes.
- the kernel's K/V loop bound was misleadingly named `n_heads`
when it's actually the sequence length (grid is one program per
(batch, head) pair, each handling its full attention row block);
renamed to `seq_len` for clarity.
Verified via CPU interpreter mode (TRITON_INTERPRET=1): output
matches `torch.nn.functional.scaled_dot_product_attention(scale=1.0)`
(the kernel doesn't apply 1/sqrt(d_head) scaling, matching the
benchmark's own unscaled "naive_attn" reference) to within float16
tolerance, across both a square and a BATCH != N_HEADS != SEQ_LEN !=
D_HEAD test case, and a per-block isolation check confirming the
grid/stride indexing doesn't cross-contaminate between heads.
- requirements.txt: dropped a stale self-referential editable install
of this repo pinned to an old commit hash, left over from a
`pip freeze` after `pip install -e .`.
Note: the flash_attn "Lite" kernel has no bounds masking on the query
block, so SEQ_LEN must be a power of two; documented and asserted in
the wrapper.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YTA6eTmLav7wfTk8dGrDrV
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.
Summary
kernels/layer_norm_fp8.py— the kernel only stored the per-row dequant scale for row 0 (if tl.program_id(0) == 0), leaving every other row's entry in thescalestensor uninitialized. Dequantizing any row but the first (y_fp8 * scale) produced garbage. Now always stores the scale.benchmarks/bench_layernorm_fp8.py— the GB/s formula always used the BF16 byte-traffic multiplier (M*N*6) even for the FP8 line, contradicting the file's own comment (which says FP8 should beM*N*5) and inflating the reported FP8 throughput by ~20% — this is the number quoted in the README (3,531.8 GB/s). Now selects the multiplier per provider.kernels/flash_attn.py—benchmarks/bench_flash_attn.pyimportsflash_attnfrom this module, but only the low-levelflash_attn_kernelexisted; the benchmark has never been able to run (ImportError), which is presumably why the README only reports a PyTorch baseline for FlashAttention and no Triton number. Added the host-sideflash_attn(q, k, v)wrapper, mirroring the pattern used inrelu.py/layer_norm.py. While wiring it up, found two more issues that would've made the kernel fail to even compile:d_headwas used as atl.zeros/tl.arangeshape argument without being markedtl.constexpr, which Triton requires for compile-time shapes.n_headswhen it's actually the sequence length (grid = one program per(batch, head)pair); renamed toseq_len.requirements.txt— dropped a stale self-referential editable install of this repo pinned to an old commit hash (leftover from apip freezeafterpip install -e .).Note: the flash_attn "Lite" kernel has no bounds masking on the query block, so
SEQ_LENmust be a power of two — documented and asserted in the wrapper.Test plan
No CUDA GPU was available to me, so I verified all three logic fixes via Triton's CPU interpreter mode (
TRITON_INTERPRET=1) rather than skipping verification:layer_norm_fp8: reproduced the original bug (7/8 rows left uninitialized) and confirmed the fix gives every row a correct, distinct scale matching an fp32 reference layernorm.flash_attn: output matchestorch.nn.functional.scaled_dot_product_attention(scale=1.0)(the kernel has no1/sqrt(d_head)scaling, consistent with the benchmark's own unscalednaive_attnreference) within float16 tolerance, tested with both square dims andBATCH != N_HEADS != SEQ_LEN != D_HEAD, plus a per-block isolation check confirming the grid/stride indexing doesn't cross-contaminate between heads.bench_layernorm_fp8.pybyte-formula fix is pure arithmetic, checked independently.Generated by Claude Code