Repository navigation
fix(rocm): route moe_align around Triton 3.8 miscompile - #687
Open
thedandano wants to merge 1 commit into
Open
thedandano wants to merge 1 commit into
thedandano wants to merge 1 commit into
Conversation
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.
Problem
On ROCm with Triton 3.8, MoE alignment can put tokens in the wrong expert blocks without raising an error. Decode and prefill can then use the wrong expert weights. This falls under the AMD work in #541.
Cause
moe_align.pybuilds store masks fromtl.histogram. Triton 3.8's AMD backend folds comparisons against that histogram to false. The compiler fix landed in triton-lang/triton#11246, but its 3.8 backport, triton-lang/triton#11577, is still open.Expected behavior
Each expert gets contiguous, padded token blocks.
expert_idsmust name the expert for each used block, andnum_tokens_post_padmust match the padded count.Fix
Use a PyTorch path with fixed-size buffers on ROCm with Triton 3.8. Keep the existing Triton path on older ROCm stacks and on CUDA. The fallback handles FreeToken's extra expert slot and invalid token IDs.
FREETOKEN_MOE_ALIGN_IMPLlets an operator force either path while diagnosing an installation.This is the follow-up to the earlier PR against @zihaomu's ROCm branch. Zihao reviewed and tested the fallback there, then suggested sending it upstream after #132 merged. Thanks for the review and for the ROCm foundation, @zihaomu.
Testing
Earlier hardware A/B testing, reported on the first PR: RX 7800 XT (
gfx1101), ROCm 10.0, PyTorch 2.11.0+rocm10.0.0, Triton 3.8.0. On the same container and GPU,pytest tests/engine/ tests/moe/aborted with a GPU memory fault after 14 tests without the fix. With the fix, it finished at 218 passed, 3 unrelated failures, and 9 skipped.tests/kernels/test_moe_align.pyhad 44 passes. These runs were on the earlier foundation branch, not the current patch againstmain.@zihaomu also tested the first PR on an R9700 with Triton 3.7.1: 24 alignment tests passed, including graph capture and replay. Their review led to the version gate and installation fix; current
mainalready allows Triton 3.8.On Fedora 44 with a Ryzen 7 7800X3D, PyTorch 2.13.0+cu130, and Triton 3.7.1:
PYTHONPATH=python python -m pytest -q --confcutdir=tests/kernels tests/kernels/test_moe_align.py-> 31 passed, 2 GPU tests skipped because this checkout has no GPU device access.python -m ruff check python/freetoken/kernel/triton/moe_align.py tests/kernels/test_moe_align.py-> passed.git diff --check-> passed.