Fix DeepSeek4HyperHead compilation error - #4778
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces the DeepSeek4HyperHead module to support DeepSeek V4 hyper heads, which reduces the expansion rate dimension and applies a series of dense layers. This head is integrated into the decoder initialization and call paths when mhc_expansion_rate > 1 and the decoder block is DEEPSEEK4. Feedback on these changes highlights two main issues: first, accessing config.mhc_expansion_rate directly can raise an AttributeError on configurations where it is not defined, so using getattr is recommended; second, the DenseGeneral layers in DeepSeek4HyperHead should be configured with proper sharding, precision, and offloading parameters to prevent potential out-of-memory (OOM) errors and ensure consistency.
| self.is_gemma4 = self.config.decoder_block == DecoderBlockType.GEMMA4 | ||
| self.is_gemma4_small = self.config.decoder_block == DecoderBlockType.GEMMA4_SMALL | ||
|
|
||
| if config.mhc_expansion_rate > 1 and config.decoder_block == DecoderBlockType.DEEPSEEK4: |
There was a problem hiding this comment.
Accessing config.mhc_expansion_rate directly will raise an AttributeError for any model configuration that does not define mhc_expansion_rate (such as standard LLaMA or Gemma models). Using getattr(config, "mhc_expansion_rate", 1) prevents this crash and safely defaults to 1 when the attribute is missing.
| if config.mhc_expansion_rate > 1 and config.decoder_block == DecoderBlockType.DEEPSEEK4: | |
| if getattr(config, "mhc_expansion_rate", 1) > 1 and config.decoder_block == DecoderBlockType.DEEPSEEK4: |
| linears.DenseGeneral( | ||
| in_features_shape=self.dim, | ||
| out_features_shape=self.dim, | ||
| dtype=self.dtype, | ||
| weight_dtype=self.weight_dtype, | ||
| rngs=self.rngs, | ||
| ) |
There was a problem hiding this comment.
The DenseGeneral layers inside DeepSeek4HyperHead are initialized without specifying logical kernel_axes, shard_mode, matmul_precision, and parameter_memory_host_offload. Without these, the weight matrices will not be sharded (replicated instead), which can lead to high memory usage and potential Out-Of-Memory (OOM) errors during large-scale training. Additionally, the layers will not respect the user's configuration for sharding, precision, and offloading. Specifying these parameters ensures proper FSDP sharding and consistency with the rest of the model.
linears.DenseGeneral(
in_features_shape=self.dim,
out_features_shape=self.dim,
dtype=self.dtype,
weight_dtype=self.weight_dtype,
kernel_axes=("embed", None),
shard_mode=config.shard_mode,
matmul_precision=config.matmul_precision,
parameter_memory_host_offload=config.parameter_memory_host_offload,
rngs=self.rngs,
)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Fix DeepSeek4HyperHead compilation error