Skip to content

DSV4: Add mhc_split_axis_contraction to contract the mHC rate and embed axes separately - #4912

Open
systalyze-ai wants to merge 1 commit into
AI-Hypercomputer:mainfrom
systalyze-ai:upstream-pr/mhc-split-axis-contraction
Open

DSV4: Add mhc_split_axis_contraction to contract the mHC rate and embed axes separately#4912
systalyze-ai wants to merge 1 commit into
AI-Hypercomputer:mainfrom
systalyze-ai:upstream-pr/mhc-split-axis-contraction

Conversation

@systalyze-ai

Copy link
Copy Markdown

Description

Adds mhc_split_axis_contraction, an opt-in path that keeps the mHC rate and TP-sharded embedding axes separate through RMS normalization and the fused alpha projection. The split contractions replace flatten-induced activation gathers with reductions over the sharded embedding axis.

When enabled, the checkpoint layout of the mHC parameters changes: the norm scale uses (k, dim) instead of (k * dim,), and the alphas use (k, dim, n) instead of (k * dim, n). Their initialized values equal the flat initialization reshaped, so converting a flag-off checkpoint is a pure reshape.

The flag defaults to false, preserving the existing RMSNorm module, parameter shapes, RNG draw order, and contraction path. DeepSeek-V4 Hugging Face conversion reshapes alpha weights for either layout, and Muon contracts both split input axes.

Performance

Step time was 7.711 s/step with mhc_split_axis_contraction disabled and 6.811 s/step with it enabled, a 1.13x result.

Reproduction setup: v6e-128, DeepSeek-V4-Flash 284B, LoRA fine-tuning, ici_expert_parallelism=16, ici_tensor_parallelism=8, ici_fsdp_parallelism=1, max_target_length=16384, per_device_batch_size=0.125 (global batch 16), LIBTPU_INIT_ARGS=--xla_tpu_scoped_vmem_limit_kib=98304, dynamic-splash attention path enabled, with mhc_split_axis_contraction toggled.

Tests

  • JAX_PLATFORMS=cpu pytest -q tests/unit/mhc_test.py -k split_axis — 3 passed; the split path matches Sinkhorn and mHC-lite outputs, and its initialized parameters are exact reshapes of the flat layout.
  • JAX_PLATFORMS=cpu pytest -q tests/unit/mhc_test.py — 27 passed, 2 skipped.
  • JAX_PLATFORMS=cpu pytest -q tests/unit/param_mapping_test.py tests/unit/muon_utils_test.py — 53 passed; split-layout Hugging Face conversion round-trips and Muon uses the intended contraction axes.

Checklist

Before submitting this PR, please make sure (put X in square brackets):

  • I have performed a self-review of my code. For an optional AI review, add the gemini-review label.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed, including adding new documentation pages to the relevant Table of Contents (toctree directive) as explained in our documentation.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the mhc_split_axis_contraction configuration option, which allows contracting the mHC rate and embedding axes separately instead of flattening them, thereby avoiding all-gathering the TP-sharded embedding dimension. To support this, a new _SplitAxesRMSNorm layer is added, and the parameter shapes, initializations, and matmuls in ManifoldConstrainedHyperConnections are adjusted accordingly. Additionally, Muon optimizer utilities and checkpoint conversion hooks are updated, and comprehensive unit tests are introduced. The review feedback suggests converting input tensors to numpy arrays before accessing .shape in mhc_concat_fn to prevent potential AttributeErrors, and using the standard [...] syntax instead of get_value() on nnx.Param for consistency and compatibility.

if len(input_tensors) != 3:
raise ValueError(f"mhc_concat_fn expected 3 tensors (pre, post, res), got {len(input_tensors)}")
tensors = [np.asarray(t) for t in input_tensors]
tensors = [np.asarray(t).reshape((-1, t.shape[-1])) for t in input_tensors]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Accessing t.shape directly on the elements of input_tensors before converting them to numpy arrays can raise an AttributeError if any element is a list or other non-array-like object. It is safer and more robust to convert to a numpy array first and then access .shape on the converted array.

Suggested change
tensors = [np.asarray(t).reshape((-1, t.shape[-1])) for t in input_tensors]
tensors = [arr.reshape((-1, arr.shape[-1])) for arr in map(np.asarray, input_tensors)]

Comment thread src/maxtext/layers/mhc.py
x = jnp.asarray(x, jnp.float32)
mean2 = jnp.mean(jax.lax.square(x), axis=(-2, -1), keepdims=True)
y = jnp.asarray(x * jax.lax.rsqrt(mean2 + self.epsilon), self.dtype)
scale = jnp.asarray(self.scale.get_value(), self.dtype)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using self.scale.get_value() is inconsistent with the rest of the file (and the MaxText codebase), which uses the standard [...] syntax (e.g., self.pre_alpha[...], self.mhc_norm.scale[...]) to access nnx.Param values. Additionally, get_value() is non-standard in Flax NNX and may cause compatibility issues or runtime errors depending on the Flax version. Please use self.scale[...] instead.

Suggested change
scale = jnp.asarray(self.scale.get_value(), self.dtype)
scale = jnp.asarray(self.scale[...], self.dtype)

…separately

The flattened (rate, embed) axis forces GSPMD to all-gather the activation's
TP-sharded embed dim. Contracting the axes separately keeps the activation
sharded; the weights are created in the consumed layout with values equal to
the flat init reshaped. Measured 7.711 to 6.811 s/step (1.13x).
Co-authored-by: Sudarsanan <[email protected]>
Co-authored-by: Armin <[email protected]>
Co-authored-by: utlz <[email protected]>
@systalyze-ai
systalyze-ai force-pushed the upstream-pr/mhc-split-axis-contraction branch from d936232 to 34e0d48 Compare August 17, 2026 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants