Skip to content

fix(kernel): make Triton top-k/top-p sampling exact - #329

Merged
jason-fxz merged 4 commits into
FlashML-org:mainfrom
taking-lying-flat:fix/triton-topk-correctness
Sep 3, 2026
Merged

fix(kernel): make Triton top-k/top-p sampling exact#329
jason-fxz merged 4 commits into
FlashML-org:mainfrom
taking-lying-flat:fix/triton-topk-correctness

Conversation

@taking-lying-flat

@taking-lying-flat taking-lying-flat commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • incorporate the exact single-launch fp32 histogram search from fix(kernel): exact single-launch triton top-k/top-p sampling #345
  • preserve every top-k/top-p boundary tie, matching flashinfer across renormalization, sampling, and combined top-k -> top-p filtering
  • retry the same exact kernel with one CTA per row when cooperative launch is unavailable
  • scope cooperative-launch capability by device and operation, and propagate unrelated Triton failures
  • remove the older approximate multi-launch fallback and redundant workspace/target conversions

Dropping the token-id tie clipping also removes its per-CTA tie workspace and one synchronization stage from the fused path. This keeps the final PR to one production file; no test files are included.

Validation

  • RTX A1000 tie regression covering cooperative multi-CTA and single-CTA fallback paths
  • bf16-logit cutoff ties for top-k/top-p renormalization and top-k, top-p, and combined sampling; every boundary tie remained reachable
  • tie-free mixed per-row top-k/top-p sweep against PyTorch references
  • all six fused kernel variants cross-compiled for sm_75, sm_80, sm_86, sm_89, sm_90, sm_100, and sm_120
  • Ruff, Python byte-compilation, and diff whitespace checks

Credit

This revision builds directly on #345. Thank you, @jason-fxz, for taking the original correctness report seriously, replacing the fragile estimator with a much stronger exact design, and carefully reviewing #329. The core single-launch histogram approach here is your work; the follow-up changes retain its exactness while making device handling and fallback behavior more robust.

@taking-lying-flat taking-lying-flat changed the title fix(sampling): handle Triton top-k candidate limits fix(kernel): make Triton top-k/top-p sampling exact Sep 2, 2026
@jason-fxz

Copy link
Copy Markdown
Collaborator

Thanks for folding #345 in and for the follow-up work — the per-device _num_sm, the in-kernel clamp and the single-CTA retry are all improvements, and everything passed on my side (no-tie sweep, tie stress, sm_75–sm_120 cross-compile).

I'm planning to merge this PR instead of #345.

One change before that: the tie clipping in _tie_prefix / _keep_mask. We'd rather keep the Triton path aligned with flashinfer, which keeps every boundary tie on every op (>= pivot for top-k renorm, "fewer than k strictly greater" for top-k sampling, > low for top-p). With bf16 logits exact ties at the cutoff are common, so the two paths would otherwise sample from different distributions on the same model. Could you drop the clipping and keep >= thr as in #345?

@jason-fxz

Copy link
Copy Markdown
Collaborator

LGTM.

@jason-fxz
jason-fxz merged commit 03c28d2 into FlashML-org:main Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants