[FEAT] Lightning Indexer#606
Conversation
Co-authored-by: Leon Ling <leon.ling@amd.com>
Strip Deep Sparse Attention from the sparse_attention package so it exposes only the lightning indexer: - Delete transformer_engine/jax/sparse_attention/dsa.py and its test (tests/jax/test_sparse_attention.py). - Drop DSA exports from sparse_attention/__init__.py; update the docstring. - Remove the test_sparse_attention.py invocation from ci/jax.sh. - Reword the now-stale DSA reference in test_indexer.py's reference oracle. The full indexer+DSA state is preserved on branch zain/lightning-indexer-dsa. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
||
|
|
||
| mlir.register_lowering(_score_reduce_p, _score_reduce_lowering, platform="rocm") | ||
| mlir.register_lowering(_score_reduce_p, _score_reduce_lowering, platform="cuda") |
There was a problem hiding this comment.
All three primitives (_score_reduce_p, _score_dscores_chunk_p, _score_topk_p) register lowerings for both platform="rocm" and platform="cuda" (also lines 389 and 967). The autotune configs, however, carry AMD-only meta-args (matrix_instr_nonkdim, waves_per_eu) and the kernel comments call out gfx950 / MI355X specifics. triton_extensions/utils.py::_compile_triton_hip filters HIP-only fields via dataclasses.fields(cb_hip.HIPOptions), but the CUDA compile path does not — so these config kwargs reach the kernel signature and fail at Triton compile time on NVIDIA.
Since this file is untested on CUDA and the tuning is ROCm-specific, I'd recommend restricting registration to platform="rocm" here (and at the other two sites). Per the ROCm-fork guidance, ROCm-only kernels should not silently claim CUDA support.
| T_s=4096; a larger footprint tips shared-GPU GEMMs into resource errors. | ||
| """ | ||
| B, oH, T_t = 1, 2, 32 | ||
| args = _indexer_inputs(B, oH, T_t, T_s=4096, d=32, d_c=32, H=16, d_i=32, seed=200) |
There was a problem hiding this comment.
With T_s=4096, S_PAD = _next_pow2(T_s) = 4096, and the lowering picks the single-sort kernel (_score_topk_single_kernel) since S_PAD <= _SINGLE_SORT_MAX = 4096. That means the streaming _score_topk_kernel — which uses the 1D-encoded-t_enc layout, the multi-outer streaming buffer, and the AMD-backend workaround called out in its docstring — is not exercised anywhere in the test suite. It is the more complex of the two and the more likely place for a regression.
Please add one case that pushes into the streaming path (e.g. T_s >= 8192, or drop _SINGLE_SORT_MAX via monkeypatch in a dedicated test).
| variables = mod.init(jax.random.PRNGKey(0), Q, K) | ||
| idx = mod.apply(variables, Q, K) | ||
| assert idx.shape == (B, oH, T_t, k) | ||
| assert idx.dtype == jnp.int32 |
There was a problem hiding this comment.
test_lightning_indexer_topk_mode only checks shape and dtype; a kernel returning uninitialized memory (or all-zeros from a buggy scatter path) would still pass. Please add a validity check on the indices:
assert bool((idx >= 0).all()) and bool((idx < T_s).all())Ideally also a set-based comparison against jax.lax.top_k (score-normalized, as in test_topk_matches_reference) — otherwise the module-level topk path is essentially untested for correctness.
| @@ -0,0 +1,24 @@ | |||
| # Copyright (c) 2024-2026, Advanced Micro Devices, Inc. All rights reserved. | |||
There was a problem hiding this comment.
This file is new in this PR (first commit 30faf3c1, June 2026). The AMD copyright range 2024-2026 implies AMD authorship starting in 2024, which isn't the case. Per the copyright-check convention (Yfirst must be ≥ the year the AMD copyright was first added), new files should carry a single year:
| # Copyright (c) 2024-2026, Advanced Micro Devices, Inc. All rights reserved. | |
| # Copyright (c) 2026, Advanced Micro Devices, Inc. All rights reserved. |
| @@ -0,0 +1,164 @@ | |||
| # Copyright (c) 2024-2026, Advanced Micro Devices, Inc. All rights reserved. | |||
There was a problem hiding this comment.
New file in this PR — AMD copyright range 2024-2026 should be single year 2026 (the year AMD authorship began for this file).
| # Copyright (c) 2024-2026, Advanced Micro Devices, Inc. All rights reserved. | |
| # Copyright (c) 2026, Advanced Micro Devices, Inc. All rights reserved. |
| @@ -0,0 +1,1016 @@ | |||
| # Copyright (c) 2024-2026, Advanced Micro Devices, Inc. All rights reserved. | |||
There was a problem hiding this comment.
New file in this PR — AMD copyright range 2024-2026 should be single year 2026.
| # Copyright (c) 2024-2026, Advanced Micro Devices, Inc. All rights reserved. | |
| # Copyright (c) 2026, Advanced Micro Devices, Inc. All rights reserved. |
| from . import flax | ||
| from . import quantize | ||
|
|
||
| # AMD lightning-indexer / sparse-attention staging module. |
There was a problem hiding this comment.
This file is now modified by ROCm (the sparse_attention import + __all__ entry added here) but still only carries the upstream NVIDIA copyright header (line 1). Per CLAUDE.md rule #2, an AMD copyright line should be added above the NVIDIA line:
# Copyright (c) 2026, Advanced Micro Devices, Inc. All rights reserved.
#
# Copyright (c) 2022-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
Leave the NVIDIA range untouched.
Claude reviewReviewed the lightning-indexer surface added by this PR — the sparse-attention API ( Highlights
Copyright headers: 4 files need updates — three new files use |
Claude WalkthroughIntent. Introduces a JAX lightning indexer for sparse attention on ROCm - a low-rank query/key projection followed by a fused Triton score+ReLU+weighted-H-reduction (and an optional fused top-k). The op is a staging ground for a DeepSeek-style DSA/HCA indexer; the PR description notes that when this eventually targets Key changes.
Walkthrough.
Testing. Notes for reviewers.
Generated by Claude. To request a code review, comment |
|
|
||
| The hybrid backend computes the four projections (C_q, H_q, H_k, W_o) via | ||
| ``jnp.einsum`` (which lowers to hipBLASLt bf16 GEMMs) and then hands the | ||
| results to this kernel for the score matmul + ReLU + per-(t, h) weighted |
There was a problem hiding this comment.
This comment probably more suitable for specific kernel
| from .utils import triton_call_lowering | ||
|
|
||
|
|
||
| def _autotune_disabled(): |
| run_default_fa 1 test_layer.py # it effectively always uses unfused attention | ||
| run_default_fa 1 test_sanity_import.py | ||
| run_default_fa 1 test_softmax.py | ||
| run_default_fa 1 test_indexer.py # lightning indexer ops (fused-attn agnostic) |
There was a problem hiding this comment.
nit: move up, before test_layer
Description
This PR contains:
Note that in the PR that will eventually target/land on main, only the API changes and triton kernel integration will persist, with kernels being relocated elsewhere.
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: