[perf]feat: Add MFU for Qwen3-VL dense #4753
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds FLOPs estimation for Qwen3-VL models. While the intention is good, the current implementation has a few critical issues. First, a change in FlopsCounter.__init__ introduces a regression that breaks FLOPs calculation for other vision-language models like qwen2_vl. Second, the new _estimate_qwen3_vl_flops function is implemented in a way that the vision transformer (ViT) part of the calculation is never executed due to a problem in how arguments are passed, leading to incorrect FLOPs counting for qwen3_vl. I've also pointed out an inaccuracy in a docstring. Please address these critical issues to ensure the feature works as intended without breaking existing functionality.
| return flops_achieved | ||
|
|
||
|
|
||
| def _estimate_qwen3_vl_flops(config,tokens_sum, batch_seqlens, delta_time, **kargs): |
There was a problem hiding this comment.
The **kargs argument will always be empty because the calling method FlopsCounter.estimate_flops does not accept or forward any keyword arguments. As a result, images_seqlens will always be None, and the ViT FLOPs calculation (vit_flops) will always be zero. This makes a significant part of this function's logic dead code and leads to incorrect FLOPs estimation for qwen3_vl.
To fix this, you need to update the signature of FlopsCounter.estimate_flops to accept additional arguments (e.g., **kwargs) and pass them along to the estimation function.
| ) | ||
|
|
||
| self.config = getattr(config, "text_config", config) | ||
| self.config = config |
There was a problem hiding this comment.
This change introduces a critical regression. While it is necessary for qwen3_vl to use the full config object to select the correct estimation function, it breaks FLOPs calculation for other vision-language (VL) models like qwen2_vl.
Other VL models rely on estimation functions like _estimate_qwen2_flops which expect a text_config object. By setting self.config to the full config for all models, these functions will receive the wrong config structure and fail with an AttributeError.
To fix this, we can special-case qwen3_vl to use the full config, while retaining the previous behavior for all other models. This ensures that the new functionality for qwen3_vl is added without breaking existing support for other models.
| self.config = config | |
| if config.model_type == "qwen3_vl": | |
| self.config = config | |
| else: | |
| self.config = getattr(config, "text_config", config) |
|
|
||
| def _estimate_qwen3_vit_flop(images_seqlens, config): | ||
| """ | ||
| Estimate the FLOPS of the vision encoder for Qwen2 and Qwen2.5 |
There was a problem hiding this comment.
The docstring is inaccurate. This function estimates FLOPS for the Qwen3-VL vision encoder, not Qwen2 and Qwen2.5. Updating the docstring will prevent future confusion.
| Estimate the FLOPS of the vision encoder for Qwen2 and Qwen2.5 | |
| Estimate the FLOPS of the vision encoder for Qwen3-VL |
1 participant
What does this PR do?
Add the _estimate_qwen3_vit_flop and _estimate_qwen3_vl_flops function to calculate the FLOPs of Qwen3-VL dense models. Update the test cases to verify the calculation accuracy of Qwen3-VL models.
Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI){modules}includefsdp,megatron,sglang,vllm,rollout,trainer,ci,training_utils,recipe,hardware,deployment,ray,worker,single_controller,misc,perf,model,algo,env,tool,ckpt,doc,data,cfg,reward,like[megatron, fsdp, doc]{type}is infeat,fix,refactor,chore,test[BREAKING]to the beginning of the title.[BREAKING][fsdp, megatron] feat: dynamic batchingTest
API and Usage Example
# Add code snippet or script demonstrating how to use thisDesign & Code Changes
Checklist Before Submitting
Important
Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=alwaysci-requestchannel in theverlSlack workspace. (If not accessible, please try the Feishu group (飞书群).)