Fix compute_flops_per_token for the MoE configs it claims to support - #7069
Open
vineethsaivs wants to merge 1 commit into
Open
Fix compute_flops_per_token for the MoE configs it claims to support#7069vineethsaivs wants to merge 1 commit into
vineethsaivs wants to merge 1 commit into
Conversation
compute_flops_per_token raised AttributeError on two of the three MoE families its docstring names. Mixtral has no moe_intermediate_size (its experts are intermediate_size wide), and Qwen2-MoE has no num_local_experts, so the transformers-version switch read a field the config does not declare. Read whichever spelling the config carries. The layer rule was also off by one: transformers puts the sparse block on layer i when (i + 1) % decoder_sparse_step == 0 and i is not in mlp_only_layers. Assisted-by: Claude Code
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.
What does this PR do?
compute_flops_per_tokenraisesAttributeErroron two of the three MoE families its own docstring names.Two causes:
Expert count and expert width are spelled differently per family. Mixtral counts experts in
num_local_expertsand sizes them withintermediate_size; Qwen2-MoE counts them innum_expertsand sizes them withmoe_intermediate_size. The transformers-version switch pickednum_local_expertson 5.1+, which Qwen2-MoE never declares. Now read whichever spelling the config carries.The MoE-layer rule was off by one and ignored
mlp_only_layers. transformers uses(layer_idx not in config.mlp_only_layers) and (layer_idx + 1) % config.decoder_sparse_step == 0; this usedlayer_idx % sparse_step == 0.Qwen3-MoE is unchanged by this PR (12,982,419,456 before and after), so the fix is a no-op on the path that already worked.
Before submitting
AI writing disclosure
Test
Three new tests in
TestComputeFlopsPerToken, plus the existingtest_moe_active_vs_total_expertsmirror expression updated to the corrected layer rule.Note
Low Risk
Changes only FLOPs/MFU estimation helpers and tests; training forward/backward paths are untouched, though reported MFU for some MoE configs may shift slightly where the layer rule was wrong.
Overview
Fixes
compute_flops_per_tokenfor Mixtral and Qwen2-MoE so MFU-style metrics no longer crash withAttributeErrorand layer counts match transformers.MoE expert count and width are read from whichever config fields exist (
num_local_experts/num_experts,moe_intermediate_size/intermediate_size) instead of assuming one naming scheme or a transformers version gate. Routed-expert FLOPs and which layers count as sparse now follow transformers:(layer_idx + 1) % decoder_sparse_step == 0, respectmlp_only_layers, and default sparse step to 1 when unset.Tests add coverage for both config spellings, Mixtral expert sizing via
intermediate_size, and sparse-step / MLP-only layering; the existing active-experts test is updated to the corrected layer rule.Reviewed by Cursor Bugbot for commit 68e09c1. Bugbot is set up for automated code reviews on this repo. Configure here.