[fix] fix determine_num_fused_shared_experts #7180
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @Alcanderian, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses a potential issue in the logic used to determine if shared experts fusion can be enabled for Deepseek V2 models. It refactors the relevant function to make the conditions for enabling/disabling fusion more explicit and easier to understand, ensuring it's only activated when the necessary hardware and configuration requirements are met.
Highlights
- Logic Refactor: Refactored the
determine_num_fused_shared_expertsfunction to simplify and clarify the conditions under which shared experts fusion is enabled or disabled for Deepseek V2 models. - Conditional Checks: Consolidated the checks for hardware requirements (CUDA, SM version >= 90) and model configuration (architecture, expert count, specific modes like deepep/ep moe) into a clearer flow.
- Improved Logging: Added a specific reason for disabling shared experts fusion when conditions are not met.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
/gemini review |
There was a problem hiding this comment.
Code Review
The refactoring of determine_num_fused_shared_experts significantly improves the clarity and correctness of the logic for enabling or disabling shared experts fusion. The new structure correctly respects the disable_shared_experts_fusion global flag, ensuring that if fusion is explicitly disabled via global arguments, this decision is not overridden by other conditions within the function. This change enhances the robustness and predictability of the shared experts fusion behavior.
|
+1 we should refactor this, o/w it seems the disable flag does not work :( (and the logic is a tiny bit confusing) I will close my #6911 in favor of this PR |
Motivation
Modifications
Checklist