fix(moe): correct MoE grouped-matmul graphs to match hipDNN contract - #40
Closed
SamuelReeder wants to merge 1 commit into
Closed
fix(moe): correct MoE grouped-matmul graphs to match hipDNN contract#40SamuelReeder wants to merge 1 commit into
SamuelReeder wants to merge 1 commit into
Conversation
All MoE graphs in moe.tar.gz and cudnn_bench_moe.tar.gz failed opgraph deserialization. Root causes: - Tensors were flattened; hipDNN's MoE node requires rank-3 shapes ([1, T, K] / [E, K, N] / [E, 1, 1]). - first_token_offset used an E+1-length cumulative-boundary array; hipDNN expects the E-length per-expert start-offset array. - top_k was set to the real routing value on NONE/GATHER nodes, but those modes require top_k == 0 (SCATTER only). - GATHER-mode dgrad nodes carried a token_ks input inherited from their paired SCATTER forward node; GATHER forbids token_ks. - token_index/token_ks used the activation dtype instead of INT32. - Some graphs had a disconnected router-GEMM component; hipDNN requires one connected component per graph, so these were split into separate *__router.json files. - SCATTER-mode output row count used the reduced final-token count; hipDNN's contract requires the dispatched-token count. Excluded 21 wgrad graphs whose token/doutput row counts mismatch (pre- vs post-scatter counts) -- MoeGroupedMatmulBwdAttributes has no scatter/token_index support today, so this isn't fixable by reshaping. Result: 191/191 graphs (165 moe.tar.gz + 26 cudnn_bench_moe.tar.gz) pass from_json + validate + build_operation_graph.
Collaborator
Author
|
Superseded — the moe fix is now part of #39 (norm + moe on one branch), no need for a separate PR for these DVC-only changes. |
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
All MoE grouped-matmul graphs in
Workloads/headline/moe.tar.gzandWorkloads/microbench/cudnn_bench_moe.tar.gzfailed opgraph-leveldeserialization against hipDNN. This corrects the graphs to match hipDNN's
MoE grouped-matmul node contract.
Risk assessment
Low. This change updates two DVC-tracked workload archives and their
internal manifests. It does not change hipDNN or provider source code.
Testing summary
tools/check_deserialize.py --level opgraph(built hipDNN,from_json+validate+build_operation_graph): 191/191 graphs pass,fail=0.non-INT32 routing tensors, or a non-zero
top_koutsideSCATTERmode.Testing checklist
Technical changes
(
[1, T, K]/[E, K, N]/[E, 1, 1]).first_token_offsetfrom anE+1-length cumulative-boundaryarray to the
E-length per-expert start-offset array hipDNN expects.top_konNONE/GATHERmode nodes (onlySCATTERuses it).token_ksinput incorrectly carried byGATHER-modedgradnodes (forbidden in that mode).
token_index/token_kstensors to INT32.*__router.jsonfiles (hipDNN requires one connected component per graph).SCATTER-mode output row count to the dispatched-token count.wgradgraphs whosetoken/doutputrow counts mismatch(pre- vs post-scatter counts); the current backward node has no
scatter/token_index support, so this isn't a shape fix. Documented as a
known gap in the manifest.