fast::rms_norm: accept Option<&Array> for weight - #347
Open
Ogilthorp3 wants to merge 1 commit into
Open
Conversation
Aligns with the existing layer_norm_device pattern in the same file
and matches Python's mx.fast.rms_norm(x, None, eps) semantics. The
underlying C function (mlx_fast_rms_norm) already supports a null
weight pointer — it checks weight.ctx and routes to the no-affine
path internally, skipping the per-element multiply kernel pass.
Only the Rust wrapper needed updating.
Why it matters: when callers want plain RMS normalization without
an affine weight (common in attention's q/k norm step where the
weight would be all-ones), passing None saves one Metal kernel
dispatch per call. For a 40-layer LLM at 1045-token prefill with
two q/k norms per attention layer, that's ~80 saved kernel passes
per forward.
Existing call sites continue to compile via auto-derived
From<&'a Array> for Option<&'a Array>. The single owned-Array test
caller (test_rms_norm) was changed to pass &weight.
Validated:
cargo test --release -p mlx-rs --lib fast::tests -- --test-threads=1
test result: ok. 6 passed; 0 failed
cargo test --release -p mlx-rs --lib nn::normalization -- --test-threads=1
test result: ok. 5 passed; 0 failed
Discovered while investigating a 2x LM-prefill perf gap between
mlx-rs and Python's mlx_lm bindings on Apple M4 Max — the qwen3_5
LinearAttention's q/k norm path was forced to construct an ones
array and call rms_norm with it, then multiply by a scale array
afterward, where Python passes None and applies a scalar multiply
inline. Microbenches and per-layer profiling confirmed parity at
the per-op level; this signature gap was one of the structural
divergences from Python's API.
Author
|
Hi — friendly check-in on this one (open ~7 days, MERGEABLE, no CI runs yet). Happy to address review feedback or rebase if needed. The downstream context: removes a redundant Metal kernel pass per call, aligns the Rust signature with the existing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fast::rms_normcurrently requires a non-optionalweight: impl AsRef<Array>. This PR changes it toweight: impl Into<Option<&'a Array>>, matching:fast::layer_normpattern in the same file (weight: impl Into<Option<&'a Array>>).mx.fast.rms_norm(x, None, eps)semantics — whenweightis None, the kernel skips the per-element multiply.Why
The underlying C function
mlx_fast_rms_normalready supports a null weight (it checksweight.ctxand routes to the no-affine path internally, skipping the multiply kernel pass). Only the Rust wrapper needed updating to expose this.When callers want plain RMS normalization without an affine weight — common in attention's q/k norm step where the weight would otherwise be all-ones — passing
Nonesaves one Metal kernel dispatch per call. Concrete impact on a 40-layer LLM at long-context prefill: ~80 saved kernel passes per forward (two q/k norms × 40 attention layers).Backward compatibility
Existing callers continue to compile via the auto-derived
From<&'a Array> for Option<&'a Array>blanket impl. The single owned-Array test caller intest_rms_normwas updated to pass&weight.Validation
The serial flag is needed to avoid the known Metal command-buffer assertion when running tests in parallel on M-series; that's pre-existing behavior, not introduced by this PR.
Discovery context
Found while investigating a 2× LM-prefill performance gap between mlx-rs and Python's
mlx_lmbindings on Apple M4 Max. Per-op microbenches showed exact parity at the FFI boundary, but the qwen3_5 LinearAttention's q/k norm path was constructing an ones array and calling rms_norm with it, then multiplying by a scale array afterward — where Python's qwen3_5 implementation passes None and applies a scalar multiply inline. This signature gap was one of the structural divergences from the Python API.Companion application-side fix in our downstream consumer:
```rust
// before
let qk_ones = self.qk_ones_weight.as_dtype(qkv_dtype)?;
let q_norm = mlx_rs::fast::rms_norm(&q, &qk_ones, 1e-6)?;
let q = q_norm.multiply(&q_scale)?;
// after (matches Python: q = inv_scale**2 * fast.rms_norm(q, None, 1e-6))
let q_norm = mlx_rs::fast::rms_norm(&q, None, 1e-6)?;
let q = q_norm.multiply(&q_scale)?;
```