[Bugfix] glm4v_moe bridge: import GPTModelProvider instead of removed Qwen3MoEModelProvider - #92
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Glm4vMoeVLModelProvider to inherit from GPTModelProvider instead of Qwen3MoEModelProvider. It also introduces several new configuration parameters for the model and MoE setup, including normalization, bias, dropout, autocast dtype, token dispatcher type, and permute fusion. The reviewer suggests dynamically retrieving the hidden_dropout value from the Hugging Face text_config instead of hardcoding it to 0.0 to ensure custom configuration settings are respected.
| normalization="RMSNorm", | ||
| gated_linear_unit=True, | ||
| add_bias_linear=False, | ||
| hidden_dropout=0.0, |
There was a problem hiding this comment.
Instead of hardcoding hidden_dropout=0.0, it is better to retrieve it from the Hugging Face text_config using getattr(text_config, "hidden_dropout", 0.0). This ensures that any custom dropout settings specified in the model configuration are respected during training/fine-tuning.
| hidden_dropout=0.0, | |
| hidden_dropout=getattr(text_config, "hidden_dropout", 0.0), |
… Qwen3MoEModelProvider Sync slime acac6616 (#1979) glm4v_moe.py change. Our megatron-bridge is now radixark@bridge (0.5.0), where megatron.bridge.models.qwen.qwen_provider was removed; Qwen3MoEModelProvider no longer exists. The eager import in slime_plugins/megatron_bridge/__init__.py therefore crashed every bridge-mode job at startup (ModuleNotFoundError), including non-GLM models such as Qwen3-VL. Switch the base class to the stable megatron.bridge.models.gpt_provider. GPTModelProvider (present in both radixark and upstream 0.5.0) and set the few config fields the Qwen base used to supply implicitly (normalization=RMSNorm, add_bias_linear=False, hidden_dropout=0.0, autocast_dtype, moe_token_dispatcher_type=alltoall, moe_permute_fusion=True). vime's file was byte-identical to slime acac6616~1, so this reproduces exactly that commit's glm4v_moe.py hunks; the file is now byte-identical to slime HEAD. No Dockerfile change needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai>
4a397e4 to
21f6144
Compare
|
LGTM |
… Qwen3MoEModelProvider (#92) Sync slime acac6616 (#1979) glm4v_moe.py change. Our megatron-bridge is now radixark@bridge (0.5.0), where megatron.bridge.models.qwen.qwen_provider was removed; Qwen3MoEModelProvider no longer exists. The eager import in slime_plugins/megatron_bridge/__init__.py therefore crashed every bridge-mode job at startup (ModuleNotFoundError), including non-GLM models such as Qwen3-VL. Switch the base class to the stable megatron.bridge.models.gpt_provider. GPTModelProvider (present in both radixark and upstream 0.5.0) and set the few config fields the Qwen base used to supply implicitly (normalization=RMSNorm, add_bias_linear=False, hidden_dropout=0.0, autocast_dtype, moe_token_dispatcher_type=alltoall, moe_permute_fusion=True). vime's file was byte-identical to slime acac6616~1, so this reproduces exactly that commit's glm4v_moe.py hunks; the file is now byte-identical to slime HEAD. No Dockerfile change needed. Signed-off-by: aoshen02 <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
slime_plugins/megatron_bridge/glm4v_moe.pyhard-importsQwen3MoEModelProviderfrommegatron.bridge.models.qwen.qwen_provider. That symbol only existed in the oldfzyzcjy@dev_rlbridge fork (0.4.0rc0). It was removed/relocated in bridge 0.5.0 (radixark@bridge— what the currentdocker/Dockerfilepins, and whatvime-vllm-r3:testships).Because
slime_plugins/megatron_bridge/__init__.pyeagerly imports this module, on a 0.5.0 image theModuleNotFoundErrorcrashes the whole job at startup — even for non-GLM models that never touch this bridge.Fix
Switch the base class from the removed
Qwen3MoEModelProviderto the stableGPTModelProvider(megatron.bridge.models.gpt_provider), and inline the MoE/TransformerConfig fields that the Qwen3 provider used to supply (normalization="RMSNorm",add_bias_linear=False,hidden_dropout=0.0,autocast_dtype,moe_token_dispatcher_type="alltoall",moe_permute_fusion=True).This mirrors upstream slime's fix (slime #1979, commit
acac6616). After this changeglm4v_moe.pyis byte-identical to slime HEAD.Why this lever (not a Dockerfile revert)
Reverting the bridge pin to fzyzcjy would make the import resolve but reintroduces the Qwen3-VL weight-sync
linear_projKeyError that 0.5.0 fixes. The correct fix is in code, on the stable base class.Test
py_compilepasses.🤖 Generated with Claude Code